From 765543199fc981749363c7d8751127ba4cc6ea29 Mon Sep 17 00:00:00 2001 From: wodka Date: Sun, 14 Feb 2016 18:48:42 +0100 Subject: [PATCH] add multiservice logic add deprecation warnings add services support to AfterSetup add tests fix https://github.com/schmittjoh/JMSDiExtraBundle/issues/232 --- Annotation/AfterSetup.php | 2 + Annotation/InjectParams.php | 3 + .../Compiler/AnnotationConfigurationPass.php | 6 +- Exception/InvalidParentException.php | 9 ++ Metadata/ClassMetadata.php | 125 ++++++++++++++-- Metadata/Driver/AnnotationDriver.php | 55 ++++--- Metadata/MetadataConverter.php | 136 ++++++++++++------ Resources/doc/annotations.rst | 12 +- .../Bundle/TestBundle/Service/Multi.php | 58 ++++++++ Tests/Functional/MultiServiceInjectTest.php | 47 ++++++ Tests/Functional/config/default.yml | 1 + Tests/Metadata/ClassMetadataTest.php | 11 +- .../Metadata/Driver/AnnotationDriverTest.php | 58 +++++++- .../Metadata/Driver/Fixture/MultiService.php | 31 ++++ 14 files changed, 466 insertions(+), 88 deletions(-) create mode 100644 Exception/InvalidParentException.php create mode 100644 Tests/Functional/Bundle/TestBundle/Service/Multi.php create mode 100644 Tests/Functional/MultiServiceInjectTest.php create mode 100644 Tests/Metadata/Driver/Fixture/MultiService.php diff --git a/Annotation/AfterSetup.php b/Annotation/AfterSetup.php index 42cf331..180afcb 100644 --- a/Annotation/AfterSetup.php +++ b/Annotation/AfterSetup.php @@ -10,4 +10,6 @@ */ final class AfterSetup { + /** @var array */ + public $services; } diff --git a/Annotation/InjectParams.php b/Annotation/InjectParams.php index 4527768..a7bac7e 100644 --- a/Annotation/InjectParams.php +++ b/Annotation/InjectParams.php @@ -26,4 +26,7 @@ final class InjectParams { /** @var array */ public $params = array(); + + /** @var array */ + public $services; } diff --git a/DependencyInjection/Compiler/AnnotationConfigurationPass.php b/DependencyInjection/Compiler/AnnotationConfigurationPass.php index 854c6ae..50b02be 100644 --- a/DependencyInjection/Compiler/AnnotationConfigurationPass.php +++ b/DependencyInjection/Compiler/AnnotationConfigurationPass.php @@ -55,14 +55,12 @@ public function process(ContainerBuilder $container) if (null === $metadata = $factory->getMetadataForClass($className)) { continue; } + if (null === $metadata->getOutsideClassMetadata()->id) { continue; } - if ( ! $metadata->getOutsideClassMetadata()->isLoadedInEnvironment($container->getParameter('kernel.environment'))) { - continue; - } - foreach ($converter->convert($metadata) as $id => $definition) { + foreach ($converter->convert($metadata, $container->getParameter('kernel.environment')) as $id => $definition) { $container->setDefinition($id, $definition); } } diff --git a/Exception/InvalidParentException.php b/Exception/InvalidParentException.php new file mode 100644 index 0000000..ce0b89d --- /dev/null +++ b/Exception/InvalidParentException.php @@ -0,0 +1,9 @@ +id)) { + $this->id = $service['id']; + $this->parent = @$service['parent']; + $this->public = @$service['public']; + $this->scope = @$service['scope']; + $this->abstract = @$service['abstract']; + $this->environments = @$service['environments']; + // TODO update call for other tags (there are several pull requests) + } + + $this->services[$service['id']] = $service; + } + + /** + * @return bool + */ + public function hasServices() + { + return !empty($this->services); + } + + /** + * get list of defined services, use fallback of original fields + * + * @return array[] + */ + public function getServices() + { + // TODO remove fallback for next major version + if (empty($this->services) || !isset($this->services[$this->id])) { + $this->services[] = array( + 'id' => $this->id, + 'parent' => $this->parent, + 'public' => $this->public, + 'scope' => $this->scope, + 'abstract' => $this->abstract, + 'environments' => $this->environments, + ); + } + + return $this->services; + } + + /** + * @deprecated this is handled on service level + * + * @param string $env * @return bool */ public function isLoadedInEnvironment($env) @@ -78,9 +185,7 @@ public function serialize() $this->initMethod, parent::serialize(), $this->environments, - $this->decorates, - $this->decoration_inner_name, - $this->deprecated, + $this->services, )); } @@ -106,9 +211,7 @@ public function unserialize($str) $this->initMethod, $parentStr, $this->environments, - $this->decorates, - $this->decoration_inner_name, - $this->deprecated, + $this->services, ) = $data; parent::unserialize($parentStr); diff --git a/Metadata/Driver/AnnotationDriver.php b/Metadata/Driver/AnnotationDriver.php index 3efc9fa..988de10 100644 --- a/Metadata/Driver/AnnotationDriver.php +++ b/Metadata/Driver/AnnotationDriver.php @@ -72,33 +72,41 @@ public function loadMetadataForClass(\ReflectionClass $class) foreach ($this->reader->getClassAnnotations($class) as $annot) { if ($annot instanceof Service) { + $service = array(); if (null === $annot->id) { - $metadata->id = $this->namingStrategy->classToServiceName($className); + $service['id'] = $this->namingStrategy->classToServiceName($className); } else { - $metadata->id = $annot->id; + $service['id'] = $annot->id; } - $metadata->parent = $annot->parent; - $metadata->public = $annot->public; - $metadata->scope = $annot->scope; - $metadata->abstract = $annot->abstract; - $metadata->decorates = $annot->decorates; - $metadata->decoration_inner_name = $annot->decoration_inner_name; - $metadata->deprecated = $annot->deprecated; + $service['parent'] = $annot->parent; + $service['public'] = $annot->public; + $service['scope'] = $annot->scope; + $service['abstract'] = $annot->abstract; + $service['environments'] = $annot->environments; + $service['decorates'] = $annot->decorates; + $service['decoration_inner_name'] = $annot->decoration_inner_name; + $service['deprecated'] = $annot->deprecated; + + $metadata->addService($service); } else if ($annot instanceof Tag) { $metadata->tags[$annot->name][] = $annot->attributes; } else if ($annot instanceof Validator) { // automatically register as service if not done explicitly - if (null === $metadata->id) { - $metadata->id = $this->namingStrategy->classToServiceName($className); + if (!$metadata->hasServices()) { + $metadata->addService(array( + 'id' => $this->namingStrategy->classToServiceName($className) + )); } $metadata->tags['validator.constraint_validator'][] = array( 'alias' => $annot->alias, ); } else if ($annot instanceof AbstractDoctrineListener) { - if (null === $metadata->id) { - $metadata->id = $this->namingStrategy->classToServiceName($className); + if (!$metadata->hasServices()) { + $metadata->addService(array( + 'id' => $this->namingStrategy->classToServiceName($className) + )); } foreach ($annot->events as $event) { @@ -110,8 +118,10 @@ public function loadMetadataForClass(\ReflectionClass $class) ); } } else if ($annot instanceof FormType) { - if (null === $metadata->id) { - $metadata->id = $this->namingStrategy->classToServiceName($className); + if (!$metadata->hasServices()) { + $metadata->addService(array( + 'id' => $this->namingStrategy->classToServiceName($className) + )); } $alias = $annot->alias; @@ -126,8 +136,10 @@ public function loadMetadataForClass(\ReflectionClass $class) 'alias' => $alias, ); } else if ($annot instanceof MetadataProcessorInterface) { - if (null === $metadata->id) { - $metadata->id = $this->namingStrategy->classToServiceName($className); + if (!$metadata->hasServices()) { + $metadata->addService(array( + 'id' => $this->namingStrategy->classToServiceName($className) + )); } $annot->processMetadata($metadata); @@ -187,7 +199,7 @@ public function loadMetadataForClass(\ReflectionClass $class) if ('__construct' === $name) { $metadata->arguments = $params; } else { - $metadata->methodCalls[] = array($name, $params); + $metadata->methodCalls[] = array($name, $params, $annot->services); } } else if ($annot instanceof LookupMethod) { $hasInjection = true; @@ -208,8 +220,11 @@ public function loadMetadataForClass(\ReflectionClass $class) throw new \RuntimeException(sprintf('The init method "%s::%s" must be public.', $method->class, $method->name)); } - $metadata->initMethod = $method->name; - $metadata->initMethods[] = $method->name; + $metadata->initMethods[] = [ + $method->name, + array(), // parameters - to be in line with methodCalls + $annot->services + ]; } else if ($annot instanceof MetadataProcessorInterface) { if (null === $metadata->id) { $metadata->id = $this->namingStrategy->classToServiceName($className); diff --git a/Metadata/MetadataConverter.php b/Metadata/MetadataConverter.php index 2010413..9cb7173 100644 --- a/Metadata/MetadataConverter.php +++ b/Metadata/MetadataConverter.php @@ -18,8 +18,8 @@ namespace JMS\DiExtraBundle\Metadata; +use JMS\DiExtraBundle\Exception\InvalidParentException; use JMS\DiExtraBundle\Exception\InvalidAnnotationException; -use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\DefinitionDecorator; use Symfony\Component\DependencyInjection\Definition; use Metadata\ClassHierarchyMetadata; @@ -30,75 +30,123 @@ class MetadataConverter * Converts class hierarchy metadata to definition instances. * * @param ClassHierarchyMetadata $metadata + * @param string $environment * @return array an array of Definition instances */ - public function convert(ClassHierarchyMetadata $metadata) + public function convert(ClassHierarchyMetadata $metadata, $environment = null) { static $count = 0; $definitions = array(); + /** @var ClassMetadata $previous */ $previous = null; + /** @var ClassMetadata $classMetadata */ foreach ($metadata->classMetadata as $classMetadata) { - if (null === $previous && null === $classMetadata->parent) { - $definition = new Definition(); - } else { - $definition = new DefinitionDecorator( - $classMetadata->parent ?: $previous->id - ); - } + foreach ($classMetadata->getServices() as $service) { + if (isset($environment) + && isset($service['environments']) + && sizeof($service['environments']) > 0 + && !in_array($environment, $service['environments']) + ) { + continue; + } - $definition->setClass($classMetadata->name); - if (null !== $classMetadata->scope) { - $definition->setScope($classMetadata->scope); - } - if (null !== $classMetadata->public) { - $definition->setPublic($classMetadata->public); - } - if (null !== $classMetadata->abstract) { - $definition->setAbstract($classMetadata->abstract); - } - if (null !== $classMetadata->arguments) { - $definition->setArguments($classMetadata->arguments); - } + if (null === $previous && !isset($service['parent'])) { + $definition = new Definition(); + } else { + if (!isset($service['parent']) && sizeof($previous->getServices()) > 1) { + throw new InvalidParentException('there are multiple services on '.$classMetadata->name); + } - $definition->setMethodCalls($classMetadata->methodCalls); - $definition->setTags($classMetadata->tags); - $definition->setProperties($classMetadata->properties); - - if (null !== $classMetadata->decorates) { - if (!method_exists($definition, 'setDecoratedService')) { - throw new InvalidAnnotationException( - sprintf( - "decorations require symfony >=2.8 on class %s", - $classMetadata->name - ) + $definition = new DefinitionDecorator( + @$service['parent'] ?: $previous->id ); } - $definition->setDecoratedService($classMetadata->decorates, $classMetadata->decoration_inner_name); - } + if (!isset($service['id'])) { + $service['id'] = '_jms_di_extra.unnamed.service_' . $count++; + } - if (null !== $classMetadata->deprecated && method_exists($definition, 'setDeprecated')) { - $definition->setDeprecated(true, $classMetadata->deprecated); - } + $definition->setClass($classMetadata->name); + if (isset($service['scope'])) { + if (!method_exists($definition, 'setScope')) { + throw new \RuntimeException('service scopes are not available on your Symfony version.'); + } + $definition->setScope($service['scope']); + } + if (isset($service['public'])) { + $definition->setPublic($service['public']); + } + if (isset($service['abstract'])) { + $definition->setAbstract($service['abstract']); + } + if (null !== $classMetadata->arguments) { + $definition->setArguments($classMetadata->arguments); + } - if (null === $classMetadata->id) { - $classMetadata->id = '_jms_di_extra.unnamed.service_'.$count++; - } + $definition->setMethodCalls($this->reduceMethodCalls($classMetadata->methodCalls, $service['id'])); + $definition->setTags($classMetadata->tags); + $definition->setProperties($classMetadata->properties); + + if (isset($service['decorates'])) { + if (!method_exists($definition, 'setDecoratedService')) { + throw new InvalidAnnotationException( + sprintf( + "decorations require symfony >=2.8 on class %s", + $classMetadata->name + ) + ); + } + + $definition->setDecoratedService($service['decorates'], $service['decoration_inner_name']); + } + + if (isset($service['deprecated']) && method_exists($definition, 'setDeprecated')) { + $definition->setDeprecated(true, $service['deprecated']); + } if (0 !== count($classMetadata->initMethods)) { - foreach ($classMetadata->initMethods as $initMethod) { - $definition->addMethodCall($initMethod); + foreach ($this->reduceMethodCalls($classMetadata->initMethods, $service['id']) as $initMethod) { + $definition->addMethodCall($initMethod[0]); } } elseif (null !== $classMetadata->initMethod) { @trigger_error('ClassMetadata::$initMethod is deprecated since version 1.7 and will be removed in 2.0. Use ClassMetadata::$initMethods instead.', E_USER_DEPRECATED); $definition->addMethodCall($classMetadata->initMethod); } - $definitions[$classMetadata->id] = $definition; + $definitions[$service['id']] = $definition; + } + $previous = $classMetadata; } return $definitions; } + + /** + * @param $methods + * @param $serviceId + * + * @return array + */ + private function reduceMethodCalls($methods, $serviceId) + { + $reduced = array(); + + /* + * $settings is an array with 3 keys: + * 0: method name + * 1: parameters + * 2: service restriction + */ + foreach ($methods as $settings) { + if (isset($settings[2]) && !in_array($serviceId, $settings[2])) { + continue; + } + + $reduced[] = $settings; + } + + return $reduced; + } } diff --git a/Resources/doc/annotations.rst b/Resources/doc/annotations.rst index f3b3cdb..218975c 100644 --- a/Resources/doc/annotations.rst +++ b/Resources/doc/annotations.rst @@ -59,9 +59,12 @@ This marks the parameters of a method for injection: class Listener { /** - * @InjectParams({ - * "em" = @Inject("doctrine.entity_manager") - * }) + * @InjectParams( + * { + * "em" = @Inject("doctrine.entity_manager") + * }, + * services = {"limit.to.this.service"} + * ) */ public function __construct(EntityManager $em, Session $session) { @@ -93,6 +96,9 @@ If you do not explicitly define a service id, then we will generated a sensible based on the fully qualified class name for you. By default, the class will be loaded in all environments unless you explicitly specify an environment via the ``environments`` attribute. +``Note``: you can define multiple @Service annotations on one class, to then limit the injection of parameters +use the "services" attribute of @InjectParams + @Tag ~~~~ Adds a tag to the service: diff --git a/Tests/Functional/Bundle/TestBundle/Service/Multi.php b/Tests/Functional/Bundle/TestBundle/Service/Multi.php new file mode 100644 index 0000000..c299cab --- /dev/null +++ b/Tests/Functional/Bundle/TestBundle/Service/Multi.php @@ -0,0 +1,58 @@ +nice = $nice; + } + + /** + * @DI\InjectParams( + * params = { + * "worker" = @DI\Inject("foo") + * }, + * services = {"first.multi"} + * ) + * + * @DI\InjectParams( + * params = { + * "worker" = @DI\Inject("bar") + * }, + * services = {"second.multi"} + * ) + */ + public function initWorker($worker) + { + $this->worker = $worker; + } + + /** + * @DI\AfterSetup(services = {"first.multi"}) + */ + public function initFirst() + { + $this->first = true; + } +} diff --git a/Tests/Functional/MultiServiceInjectTest.php b/Tests/Functional/MultiServiceInjectTest.php new file mode 100644 index 0000000..5146f8a --- /dev/null +++ b/Tests/Functional/MultiServiceInjectTest.php @@ -0,0 +1,47 @@ +createClient(); + + $container = self::$kernel->getContainer(); + $nice = $container->get('nice'); + $foo = $container->get('foo'); + $bar = $container->get('bar'); + $first = $container->get('first.multi'); + $second = $container->get('second.multi'); + + $this->assertSame($nice, $first->nice, 'all injections without restriction should be done'); + $this->assertSame($foo, $first->worker); + $this->assertSame(true, $first->first); + + $this->assertSame($nice, $second->nice, 'all injections without restriction should be done'); + $this->assertSame($bar, $second->worker); + $this->assertSame(null, $second->first); + + $this->assertEquals(false, $container->has('third.multi'), 'should load third service only in dev'); + } + + /** + * @runInSeparateProcess + */ + public function testEnvironmentLoading() + { + // this is broken -> cannot load any other environment than test + $this->createClient( + array( + 'environment' => 'dev' + ) + ); + $container = self::$kernel->getContainer(); + + $this->assertEquals(true, $container->has('fourth.multi'), 'should load fourth service only in test'); + } +} diff --git a/Tests/Functional/config/default.yml b/Tests/Functional/config/default.yml index c04fc33..963e683 100644 --- a/Tests/Functional/config/default.yml +++ b/Tests/Functional/config/default.yml @@ -10,6 +10,7 @@ jms_di_extra: services: foo: { class: stdClass } bar: { class: stdClass } + nice: { class: stdClass } controller.hello: class: JMS\DiExtraBundle\Tests\Functional\Bundle\TestBundle\Controller\ServiceController arguments: [ "@router" ] diff --git a/Tests/Metadata/ClassMetadataTest.php b/Tests/Metadata/ClassMetadataTest.php index 5ce158a..febcc41 100644 --- a/Tests/Metadata/ClassMetadataTest.php +++ b/Tests/Metadata/ClassMetadataTest.php @@ -29,9 +29,14 @@ public function testSerializeUnserialize() $classMetadata->abstract = true; $classMetadata->public = false; $classMetadata->id = 'foo'; - $classMetadata->deprecated = true; - $classMetadata->decorates = 'test.service'; - $classMetadata->decoration_inner_name = 'old.test.service'; + $classMetadata->addService(array( + 'id' => 'foo', + 'abstract' => true, + 'public' => false, + 'deprecated' => true, + 'decorates' => 'test.service', + 'decoration_inner_name' => 'old.test.service', + )); $this->assertEquals($classMetadata, unserialize(serialize($classMetadata))); } diff --git a/Tests/Metadata/Driver/AnnotationDriverTest.php b/Tests/Metadata/Driver/AnnotationDriverTest.php index a3dc56f..ebdcf59 100644 --- a/Tests/Metadata/Driver/AnnotationDriverTest.php +++ b/Tests/Metadata/Driver/AnnotationDriverTest.php @@ -5,6 +5,7 @@ use Doctrine\Common\Annotations\AnnotationReader; use JMS\DiExtraBundle\Metadata\DefaultNamingStrategy; use JMS\DiExtraBundle\Metadata\Driver\AnnotationDriver; +use Symfony\Component\DependencyInjection\Reference; class AnnotationDriverTest extends \PHPUnit_Framework_TestCase { @@ -40,10 +41,20 @@ public function testCustomAnnotationOnClass() public function testServiceAnnotations() { $metadata = $this->getDriver()->loadMetadataForClass(new \ReflectionClass('JMS\DiExtraBundle\Tests\Metadata\Driver\Fixture\Service')); + + $services = $metadata->getServices(); + $service = array_filter(@$services['test.service'], function ($value) { return $value !== null; }); + $this->assertEquals('test.service', $metadata->id); - $this->assertEquals('test.service', $metadata->decorates); - $this->assertEquals('original.test.service', $metadata->decoration_inner_name); - $this->assertEquals('use new.test.service instead', $metadata->deprecated); + $this->assertArrayHasKey('test.service', $metadata->getServices()); + $this->assertEquals(array( + 'id' => 'test.service', + 'public' => false, + 'decorates' => 'test.service', + 'decoration_inner_name' => 'original.test.service', + 'deprecated' => 'use new.test.service instead', + 'environments' => array() + ), $service); $this->assertEquals(false, $metadata->public); } @@ -53,6 +64,47 @@ public function testCustomAnnotationOnMethod() $this->assertEquals('fancy', @$metadata->tags['omg'], 'check key and value of custom annotation'); } + public function testMultiService() + { + $metadata = $this->getDriver()->loadMetadataForClass(new \ReflectionClass('JMS\DiExtraBundle\Tests\Metadata\Driver\Fixture\MultiService')); + + $services = $metadata->getServices(); + // this is important to cleanup empty values + array_walk($services, function (&$service) { + $service = array_filter($service); + }); + + $this->assertEquals(array( + 'first.service' => array( + 'id' => 'first.service', + ), + 'second.service' => array( + 'id' => 'second.service', + ) + ), $services, 'create multiple services'); + + + $this->assertEquals( + array( + array( + 'init', + array( + 0 => new Reference('foo') + ), + array('first.service') + ), + array( + 'init', + array( + 0 => new Reference('bar') + ), + array('second.service') + ) + ), + $metadata->methodCalls, + 'service limitation for inject params was used' + ); + } private function getDriver() { diff --git a/Tests/Metadata/Driver/Fixture/MultiService.php b/Tests/Metadata/Driver/Fixture/MultiService.php new file mode 100644 index 0000000..c5ecb03 --- /dev/null +++ b/Tests/Metadata/Driver/Fixture/MultiService.php @@ -0,0 +1,31 @@ +