-
Notifications
You must be signed in to change notification settings - Fork 48
Factory service test #85
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
312de09
7aacdbb
e49fdd3
384d88f
4c7905e
16b4ece
323330e
a84f713
c138ec9
268b680
9802ce7
6d228af
569d9af
eb5bfec
1c71370
3fb9b74
5e84b5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
use PHPUnit\Framework\Constraint\IsEqual; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\Reference; | ||
use Symfony\Component\DependencyInjection\Definition; | ||
|
||
class ContainerBuilderHasFactoryConstraint extends Constraint | ||
{ | ||
|
@@ -93,7 +94,7 @@ private function evaluateFactory(ContainerBuilder $containerBuilder, $returnResu | |
/** @var Definition */ | ||
$definition = $containerBuilder->getDefinition($this->serviceId); | ||
|
||
$factory = $definition->getFactory(); | ||
$factory = $this->getFactoryData($definition); | ||
|
||
if( !is_array( $factory ) ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if ($returnResult) { | ||
|
@@ -117,7 +118,7 @@ private function evaluateFactoryClass(ContainerBuilder $containerBuilder, $retur | |
/** @var Definition */ | ||
$definition = $containerBuilder->getDefinition($this->serviceId); | ||
|
||
$factory = $definition->getFactory(); | ||
$factory = $this->getFactoryData( $definition ); | ||
|
||
list( $factoryDefinition, $factoryMethod ) = $factory; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove all spaces in function calls. Same for those in |
||
|
||
|
@@ -175,4 +176,30 @@ private function evaluateFactoryClass(ContainerBuilder $containerBuilder, $retur | |
return true; | ||
} | ||
|
||
private function getFactoryData( Definition $definition ) | ||
{ | ||
if( self::isLegacySymfonyDI() ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a dirty, but working solution ... I checked with DI 2.3, 3.0 and 3.3 and they working ... a better approach may be an abstract class with "getFactoryData" as abstract method, but this commit resolves <= 2.6 versions and variated factory syntax in yml. A test case not cover is a service with "setFactoryClass" definition ... This is pending. |
||
$factoryService = $definition->getFactoryService(); | ||
$factoryMethod = $definition->getFactoryMethod(); | ||
$factoryClass = $definition->getFactoryClass(); | ||
if( !$factoryService && !$factoryClass ) | ||
return null; | ||
|
||
return array( $factoryClass ? $factoryClass : $factoryService, $factoryMethod ); | ||
} else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No point for |
||
$factory = $definition->getFactory(); | ||
if( is_array( $factory ) ) return $factory; | ||
|
||
if( is_string( $factory ) && false !== strpos( $factory, ':' ) ) | ||
return preg_split( '/:/', $factory, 2 ); | ||
|
||
return $factory; | ||
} | ||
} | ||
|
||
|
||
static public function isLegacySymfonyDI() | ||
{ | ||
return method_exists( new Definition(), 'getFactoryService' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to create an instance here, just using the class name works too |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<?xml version="1.0" ?> | ||
<container xmlns="http://symfony.com/schema/dic/services" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> | ||
<services> | ||
<service id="factory_service"> | ||
</service> | ||
|
||
<service id="created_by_factory_service" factory-service="factory_service" factory-method="factoryMethod"> | ||
</service> | ||
</services> | ||
</container> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
services: | ||
created_with_factory_with_old_syntax: | ||
factory: ['@factory_service', 'factoryMethod'] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
<?xml version="1.0" ?> | ||
<container xmlns="http://symfony.com/schema/dic/services" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> | ||
<services> | ||
<service id="factory_service"> | ||
</service> | ||
|
||
<service id="created_by_factory_service"> | ||
<factory service="factory_service" method="factoryMethod" /> | ||
</service> | ||
</services> | ||
</container> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could check if the
Definition::getFactoryClass()
method exists to implement a pre Symfony 2.7 compatible solution.