From 3cb2d517a06664e24052d6194e84803e045c0f70 Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Tue, 12 Jun 2018 15:30:27 +0200 Subject: [PATCH 01/10] Use hashed values for method names in the CompiledContainer --- src/Compiler/Compiler.php | 20 ++++++++--- .../IntegrationTest/CompiledContainerTest.php | 34 +++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index 90b1ecd26..03cfeb7e9 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -141,6 +141,19 @@ public function compile( return $fileName; } + /** + * Use a hash to ensure that the used method names in the CompiledContainer are both unique and idempotent. + * + * @param string $prefix + * @param string $value + * + * @return string + */ + private function getHashedValue(string $prefix, string $value): string + { + return sprintf('%s%s', $prefix, md5($value)); + } + /** * @throws DependencyException * @throws InvalidDefinition @@ -148,8 +161,7 @@ public function compile( */ private function compileDefinition(string $entryName, Definition $definition) : string { - // Generate a unique method name - $methodName = str_replace('.', '', uniqid('get', true)); + $methodName = $this->getHashedValue('get', $entryName . $definition); $this->entryToMethodMapping[$entryName] = $methodName; switch (true) { @@ -260,8 +272,8 @@ public function compileValue($value) : string } if ($value instanceof Definition) { - // Give it an arbitrary unique name - $subEntryName = uniqid('SubEntry'); + $subEntryName = $this->getHashedValue('SubEntry', $value->getName() . $value); + // Compile the sub-definition in another method $methodName = $this->compileDefinition($subEntryName, $value); // The value is now a method call to that method (which returns the value) diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php index 973891796..c1fb2a8b4 100644 --- a/tests/IntegrationTest/CompiledContainerTest.php +++ b/tests/IntegrationTest/CompiledContainerTest.php @@ -9,6 +9,8 @@ use function DI\create; use DI\Definition\Exception\InvalidDefinition; use function DI\get; +use DI\Test\IntegrationTest\Definitions\NestedDefinitionsTest\AllKindsOfInjections; +use DI\Test\IntegrationTest\Definitions\NestedDefinitionsTest\Autowireable; /** * Tests specific to the compiled container. @@ -56,6 +58,38 @@ public function the_container_is_compiled_once_and_never_recompiled_after() self::assertEquals('bar', $container->get('foo')); } + /** @test */ + public function the_compiled_container_is_idempotent() + { + $compiledContainerClass1 = self::generateCompiledClassName(); + $compiledContainerClass2 = self::generateCompiledClassName(); + + $definitions = [ + 'foo' => 'bar', + AllKindsOfInjections::class => create() + ->constructor(create('stdClass')) + ->property('property', autowire(Autowireable::class)) + ->method('method', \DI\factory(function () { + return new \stdClass; + })), + ]; + + // Create a compiled container in a specific file + $builder1 = new ContainerBuilder; + $builder1->addDefinitions($definitions); + $builder1->enableCompilation(self::COMPILATION_DIR, $compiledContainerClass1); + $builder1->build(); + + // Create a second compiled container with the same configuration but in a different file + $builder2 = new ContainerBuilder; + $builder2->addDefinitions($definitions); + $builder2->enableCompilation(self::COMPILATION_DIR, $compiledContainerClass2); + $builder2->build(); + + // The method mapping of the resulting CompiledContainers should be equal + self::assertEquals($compiledContainerClass1::METHOD_MAPPING, $compiledContainerClass2::METHOD_MAPPING); + } + /** * @test * @expectedException \DI\Definition\Exception\InvalidDefinition From ecb2bbdc8983054b3c393d073899a4e0b87b2e52 Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Wed, 13 Jun 2018 10:52:25 +0200 Subject: [PATCH 02/10] Processed feedback --- src/Compiler/Compiler.php | 9 ++---- .../IntegrationTest/CompiledContainerTest.php | 32 ++++++++++++++++--- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index 03cfeb7e9..d2c92a9f3 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -143,15 +143,10 @@ public function compile( /** * Use a hash to ensure that the used method names in the CompiledContainer are both unique and idempotent. - * - * @param string $prefix - * @param string $value - * - * @return string */ private function getHashedValue(string $prefix, string $value): string { - return sprintf('%s%s', $prefix, md5($value)); + return $prefix . md5($value); } /** @@ -161,7 +156,7 @@ private function getHashedValue(string $prefix, string $value): string */ private function compileDefinition(string $entryName, Definition $definition) : string { - $methodName = $this->getHashedValue('get', $entryName . $definition); + $methodName = $this->getHashedValue('get', $entryName); $this->entryToMethodMapping[$entryName] = $methodName; switch (true) { diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php index c1fb2a8b4..7735051cc 100644 --- a/tests/IntegrationTest/CompiledContainerTest.php +++ b/tests/IntegrationTest/CompiledContainerTest.php @@ -9,8 +9,6 @@ use function DI\create; use DI\Definition\Exception\InvalidDefinition; use function DI\get; -use DI\Test\IntegrationTest\Definitions\NestedDefinitionsTest\AllKindsOfInjections; -use DI\Test\IntegrationTest\Definitions\NestedDefinitionsTest\Autowireable; /** * Tests specific to the compiled container. @@ -66,9 +64,9 @@ public function the_compiled_container_is_idempotent() $definitions = [ 'foo' => 'bar', - AllKindsOfInjections::class => create() + CompiledContainerTest\AllKindsOfInjections::class => create() ->constructor(create('stdClass')) - ->property('property', autowire(Autowireable::class)) + ->property('property', autowire(CompiledContainerTest\Autowireable::class)) ->method('method', \DI\factory(function () { return new \stdClass; })), @@ -287,3 +285,29 @@ public function __construct(AbstractClass $param) abstract class AbstractClass { } + +class AllKindsOfInjections +{ + public $property; + public $constructorParameter; + public $methodParameter; + public function __construct($constructorParameter) + { + $this->constructorParameter = $constructorParameter; + } + public function method($methodParameter) + { + $this->methodParameter = $methodParameter; + } +} +class Autowireable +{ + private $dependency; + public function __construct(AutowireableDependency $dependency) + { + $this->dependency = $dependency; + } +} +class AutowireableDependency +{ +} From bad691c0706b82d158095427fc7993a161e50ae0 Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Wed, 13 Jun 2018 15:22:40 +0200 Subject: [PATCH 03/10] Added some tests --- .../IntegrationTest/CompiledContainerTest.php | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php index 7735051cc..e406c64ba 100644 --- a/tests/IntegrationTest/CompiledContainerTest.php +++ b/tests/IntegrationTest/CompiledContainerTest.php @@ -63,19 +63,35 @@ public function the_compiled_container_is_idempotent() $compiledContainerClass2 = self::generateCompiledClassName(); $definitions = [ - 'foo' => 'bar', + 'foo' => 'barFromFoo', + 'fooReference' => \DI\get('foo'), + 'factory' => function () { + return 'barFromFactory'; + }, + 'factoryReference' => \DI\get('factory'), + 'array' => [ + 1, + 2, + 3, + 'fooBar', + ], + 'arrayValue' => \DI\value('array'), CompiledContainerTest\AllKindsOfInjections::class => create() ->constructor(create('stdClass')) ->property('property', autowire(CompiledContainerTest\Autowireable::class)) - ->method('method', \DI\factory(function () { - return new \stdClass; - })), + ->method('method', \DI\factory( + function () { + return new \stdClass; + } + ) + ), + CompiledContainerTest\Autowireable::class => \DI\autowire(), ]; // Create a compiled container in a specific file $builder1 = new ContainerBuilder; $builder1->addDefinitions($definitions); - $builder1->enableCompilation(self::COMPILATION_DIR, $compiledContainerClass1); + $builder1->enableCompilation(__DIR__, $compiledContainerClass1); $builder1->build(); // Create a second compiled container with the same configuration but in a different file From b81b6d8a21d1f3d79f3948df6075a2d7fac7dcee Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Wed, 13 Jun 2018 15:23:10 +0200 Subject: [PATCH 04/10] Use proper compilation dir --- tests/IntegrationTest/CompiledContainerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php index e406c64ba..87b0d9901 100644 --- a/tests/IntegrationTest/CompiledContainerTest.php +++ b/tests/IntegrationTest/CompiledContainerTest.php @@ -91,7 +91,7 @@ function () { // Create a compiled container in a specific file $builder1 = new ContainerBuilder; $builder1->addDefinitions($definitions); - $builder1->enableCompilation(__DIR__, $compiledContainerClass1); + $builder1->enableCompilation(self::COMPILATION_DIR, $compiledContainerClass1); $builder1->build(); // Create a second compiled container with the same configuration but in a different file From aca43d2a39071def957d19388baacde06793fce9 Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Wed, 13 Jun 2018 15:34:32 +0200 Subject: [PATCH 05/10] Add check to ensure equal entryNames point to the same methodNames as well --- src/Compiler/Compiler.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index d2c92a9f3..36e1012c7 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -157,6 +157,17 @@ private function getHashedValue(string $prefix, string $value): string private function compileDefinition(string $entryName, Definition $definition) : string { $methodName = $this->getHashedValue('get', $entryName); + + //In case an Entry is already added, the used method should be equal + if(isset($this->entryToMethodMapping[$entryName]) && $this->entryToMethodMapping[$entryName] !== $methodName){ + throw new InvalidDefinition(sprintf( + 'Entry "%s" cannot be compiled. An Entry with the same name already exists pointing to method %s(), while this one points to method %s().', + $entryName, + $this->entryToMethodMapping[$entryName], + $methodName + )); + } + $this->entryToMethodMapping[$entryName] = $methodName; switch (true) { From dbee249665d324935e4512ccc96369166c0b9914 Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Tue, 12 Jun 2018 15:30:27 +0200 Subject: [PATCH 06/10] Use hashed values for method names in the CompiledContainer --- src/Compiler/Compiler.php | 20 ++++++++--- .../IntegrationTest/CompiledContainerTest.php | 34 +++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index 90b1ecd26..03cfeb7e9 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -141,6 +141,19 @@ public function compile( return $fileName; } + /** + * Use a hash to ensure that the used method names in the CompiledContainer are both unique and idempotent. + * + * @param string $prefix + * @param string $value + * + * @return string + */ + private function getHashedValue(string $prefix, string $value): string + { + return sprintf('%s%s', $prefix, md5($value)); + } + /** * @throws DependencyException * @throws InvalidDefinition @@ -148,8 +161,7 @@ public function compile( */ private function compileDefinition(string $entryName, Definition $definition) : string { - // Generate a unique method name - $methodName = str_replace('.', '', uniqid('get', true)); + $methodName = $this->getHashedValue('get', $entryName . $definition); $this->entryToMethodMapping[$entryName] = $methodName; switch (true) { @@ -260,8 +272,8 @@ public function compileValue($value) : string } if ($value instanceof Definition) { - // Give it an arbitrary unique name - $subEntryName = uniqid('SubEntry'); + $subEntryName = $this->getHashedValue('SubEntry', $value->getName() . $value); + // Compile the sub-definition in another method $methodName = $this->compileDefinition($subEntryName, $value); // The value is now a method call to that method (which returns the value) diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php index 973891796..c1fb2a8b4 100644 --- a/tests/IntegrationTest/CompiledContainerTest.php +++ b/tests/IntegrationTest/CompiledContainerTest.php @@ -9,6 +9,8 @@ use function DI\create; use DI\Definition\Exception\InvalidDefinition; use function DI\get; +use DI\Test\IntegrationTest\Definitions\NestedDefinitionsTest\AllKindsOfInjections; +use DI\Test\IntegrationTest\Definitions\NestedDefinitionsTest\Autowireable; /** * Tests specific to the compiled container. @@ -56,6 +58,38 @@ public function the_container_is_compiled_once_and_never_recompiled_after() self::assertEquals('bar', $container->get('foo')); } + /** @test */ + public function the_compiled_container_is_idempotent() + { + $compiledContainerClass1 = self::generateCompiledClassName(); + $compiledContainerClass2 = self::generateCompiledClassName(); + + $definitions = [ + 'foo' => 'bar', + AllKindsOfInjections::class => create() + ->constructor(create('stdClass')) + ->property('property', autowire(Autowireable::class)) + ->method('method', \DI\factory(function () { + return new \stdClass; + })), + ]; + + // Create a compiled container in a specific file + $builder1 = new ContainerBuilder; + $builder1->addDefinitions($definitions); + $builder1->enableCompilation(self::COMPILATION_DIR, $compiledContainerClass1); + $builder1->build(); + + // Create a second compiled container with the same configuration but in a different file + $builder2 = new ContainerBuilder; + $builder2->addDefinitions($definitions); + $builder2->enableCompilation(self::COMPILATION_DIR, $compiledContainerClass2); + $builder2->build(); + + // The method mapping of the resulting CompiledContainers should be equal + self::assertEquals($compiledContainerClass1::METHOD_MAPPING, $compiledContainerClass2::METHOD_MAPPING); + } + /** * @test * @expectedException \DI\Definition\Exception\InvalidDefinition From 904f844acd258e6c6fd9ee3eb964cfe6cf95c9d8 Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Wed, 13 Jun 2018 10:52:25 +0200 Subject: [PATCH 07/10] Processed feedback --- src/Compiler/Compiler.php | 9 ++---- .../IntegrationTest/CompiledContainerTest.php | 32 ++++++++++++++++--- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index 03cfeb7e9..d2c92a9f3 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -143,15 +143,10 @@ public function compile( /** * Use a hash to ensure that the used method names in the CompiledContainer are both unique and idempotent. - * - * @param string $prefix - * @param string $value - * - * @return string */ private function getHashedValue(string $prefix, string $value): string { - return sprintf('%s%s', $prefix, md5($value)); + return $prefix . md5($value); } /** @@ -161,7 +156,7 @@ private function getHashedValue(string $prefix, string $value): string */ private function compileDefinition(string $entryName, Definition $definition) : string { - $methodName = $this->getHashedValue('get', $entryName . $definition); + $methodName = $this->getHashedValue('get', $entryName); $this->entryToMethodMapping[$entryName] = $methodName; switch (true) { diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php index c1fb2a8b4..7735051cc 100644 --- a/tests/IntegrationTest/CompiledContainerTest.php +++ b/tests/IntegrationTest/CompiledContainerTest.php @@ -9,8 +9,6 @@ use function DI\create; use DI\Definition\Exception\InvalidDefinition; use function DI\get; -use DI\Test\IntegrationTest\Definitions\NestedDefinitionsTest\AllKindsOfInjections; -use DI\Test\IntegrationTest\Definitions\NestedDefinitionsTest\Autowireable; /** * Tests specific to the compiled container. @@ -66,9 +64,9 @@ public function the_compiled_container_is_idempotent() $definitions = [ 'foo' => 'bar', - AllKindsOfInjections::class => create() + CompiledContainerTest\AllKindsOfInjections::class => create() ->constructor(create('stdClass')) - ->property('property', autowire(Autowireable::class)) + ->property('property', autowire(CompiledContainerTest\Autowireable::class)) ->method('method', \DI\factory(function () { return new \stdClass; })), @@ -287,3 +285,29 @@ public function __construct(AbstractClass $param) abstract class AbstractClass { } + +class AllKindsOfInjections +{ + public $property; + public $constructorParameter; + public $methodParameter; + public function __construct($constructorParameter) + { + $this->constructorParameter = $constructorParameter; + } + public function method($methodParameter) + { + $this->methodParameter = $methodParameter; + } +} +class Autowireable +{ + private $dependency; + public function __construct(AutowireableDependency $dependency) + { + $this->dependency = $dependency; + } +} +class AutowireableDependency +{ +} From 504699e5c2091dd054948feaf823fbb197445303 Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Wed, 13 Jun 2018 15:22:40 +0200 Subject: [PATCH 08/10] Added some tests --- .../IntegrationTest/CompiledContainerTest.php | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php index 7735051cc..e406c64ba 100644 --- a/tests/IntegrationTest/CompiledContainerTest.php +++ b/tests/IntegrationTest/CompiledContainerTest.php @@ -63,19 +63,35 @@ public function the_compiled_container_is_idempotent() $compiledContainerClass2 = self::generateCompiledClassName(); $definitions = [ - 'foo' => 'bar', + 'foo' => 'barFromFoo', + 'fooReference' => \DI\get('foo'), + 'factory' => function () { + return 'barFromFactory'; + }, + 'factoryReference' => \DI\get('factory'), + 'array' => [ + 1, + 2, + 3, + 'fooBar', + ], + 'arrayValue' => \DI\value('array'), CompiledContainerTest\AllKindsOfInjections::class => create() ->constructor(create('stdClass')) ->property('property', autowire(CompiledContainerTest\Autowireable::class)) - ->method('method', \DI\factory(function () { - return new \stdClass; - })), + ->method('method', \DI\factory( + function () { + return new \stdClass; + } + ) + ), + CompiledContainerTest\Autowireable::class => \DI\autowire(), ]; // Create a compiled container in a specific file $builder1 = new ContainerBuilder; $builder1->addDefinitions($definitions); - $builder1->enableCompilation(self::COMPILATION_DIR, $compiledContainerClass1); + $builder1->enableCompilation(__DIR__, $compiledContainerClass1); $builder1->build(); // Create a second compiled container with the same configuration but in a different file From a0bbb461e518a3828959fb37faff26be1bdd4501 Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Wed, 13 Jun 2018 15:23:10 +0200 Subject: [PATCH 09/10] Use proper compilation dir --- tests/IntegrationTest/CompiledContainerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/IntegrationTest/CompiledContainerTest.php b/tests/IntegrationTest/CompiledContainerTest.php index e406c64ba..87b0d9901 100644 --- a/tests/IntegrationTest/CompiledContainerTest.php +++ b/tests/IntegrationTest/CompiledContainerTest.php @@ -91,7 +91,7 @@ function () { // Create a compiled container in a specific file $builder1 = new ContainerBuilder; $builder1->addDefinitions($definitions); - $builder1->enableCompilation(__DIR__, $compiledContainerClass1); + $builder1->enableCompilation(self::COMPILATION_DIR, $compiledContainerClass1); $builder1->build(); // Create a second compiled container with the same configuration but in a different file From 8f803084c943a8de2a747e95d3bfa38407f5cbbe Mon Sep 17 00:00:00 2001 From: Menno Holtkamp Date: Wed, 13 Jun 2018 15:34:32 +0200 Subject: [PATCH 10/10] Add check to ensure equal entryNames point to the same methodNames as well --- src/Compiler/Compiler.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Compiler/Compiler.php b/src/Compiler/Compiler.php index d2c92a9f3..36e1012c7 100644 --- a/src/Compiler/Compiler.php +++ b/src/Compiler/Compiler.php @@ -157,6 +157,17 @@ private function getHashedValue(string $prefix, string $value): string private function compileDefinition(string $entryName, Definition $definition) : string { $methodName = $this->getHashedValue('get', $entryName); + + //In case an Entry is already added, the used method should be equal + if(isset($this->entryToMethodMapping[$entryName]) && $this->entryToMethodMapping[$entryName] !== $methodName){ + throw new InvalidDefinition(sprintf( + 'Entry "%s" cannot be compiled. An Entry with the same name already exists pointing to method %s(), while this one points to method %s().', + $entryName, + $this->entryToMethodMapping[$entryName], + $methodName + )); + } + $this->entryToMethodMapping[$entryName] = $methodName; switch (true) {