From d25b55d516f44d4448ffe1d636cb6d636923d542 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 tests fix https://github.com/schmittjoh/JMSDiExtraBundle/issues/232 --- Annotation/InjectParams.php | 3 + .../Compiler/AnnotationConfigurationPass.php | 6 +- Exception/InvalidParentException.php | 9 ++ Metadata/ClassMetadata.php | 133 +++++++++++++++++- Metadata/Driver/AnnotationDriver.php | 42 ++++-- Metadata/MetadataConverter.php | 115 +++++++++++---- Resources/doc/annotations.rst | 12 +- .../Bundle/TestBundle/Service/Multi.php | 49 +++++++ Tests/Functional/MultiServiceInjectTest.php | 45 ++++++ Tests/Functional/config/default.yml | 1 + .../Metadata/Driver/AnnotationDriverTest.php | 42 ++++++ .../Metadata/Driver/Fixture/MultiService.php | 31 ++++ 12 files changed, 431 insertions(+), 57 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/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) { if (empty($this->environments)) { @@ -60,6 +184,7 @@ public function serialize() $this->initMethod, parent::serialize(), $this->environments, + $this->services, )); } @@ -79,13 +204,11 @@ public function unserialize($str) $this->lookupMethods, $this->properties, $this->initMethod, - $parentStr + $parentStr, + $this->environments, + $this->services, ) = $data; - if (isset($data[12])) { - $this->environments = $data[12]; - } - parent::unserialize($parentStr); } } diff --git a/Metadata/Driver/AnnotationDriver.php b/Metadata/Driver/AnnotationDriver.php index 4ccf32b..25f9d71 100644 --- a/Metadata/Driver/AnnotationDriver.php +++ b/Metadata/Driver/AnnotationDriver.php @@ -72,30 +72,38 @@ 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; + $service['parent'] = $annot->parent; + $service['public'] = $annot->public; + $service['scope'] = $annot->scope; + $service['abstract'] = $annot->abstract; + $service['environments'] = $annot->environments; + + $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) { @@ -107,8 +115,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; @@ -123,8 +133,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); @@ -184,7 +196,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; diff --git a/Metadata/MetadataConverter.php b/Metadata/MetadataConverter.php index 9c03d70..becff14 100644 --- a/Metadata/MetadataConverter.php +++ b/Metadata/MetadataConverter.php @@ -18,6 +18,7 @@ namespace JMS\DiExtraBundle\Metadata; +use JMS\DiExtraBundle\Exception\InvalidParentException; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\DefinitionDecorator; use Symfony\Component\DependencyInjection\Definition; @@ -29,53 +30,107 @@ 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 (@$service['id'] == 'third.multi') { + //echo $environment; - $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); - } + //print_r($service); + } - $definition->setMethodCalls($classMetadata->methodCalls); - $definition->setTags($classMetadata->tags); - $definition->setProperties($classMetadata->properties); + if (isset($environment) + && isset($service['environments']) + && sizeof($service['environments']) > 0 + && !in_array($environment, $service['environments']) + ) { + continue; + } - if (null === $classMetadata->id) { - $classMetadata->id = '_jms_di_extra.unnamed.service_'.$count++; - } + 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 = new DefinitionDecorator( + @$service['parent'] ?: $previous->id + ); + } + + if (!isset($service['id'])) { + $service['id'] = '_jms_di_extra.unnamed.service_' . $count++; + } + + $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); + } + + $definition->setMethodCalls($this->reduceMethodCalls($classMetadata->methodCalls, $service['id'])); + $definition->setTags($classMetadata->tags); + $definition->setProperties($classMetadata->properties); + + if ($classMetadata->initMethod) { + throw new \RuntimeException(sprintf('You can\'t use @AfterSetup on a service.')); + } - if ($classMetadata->initMethod) { - throw new \RuntimeException(sprintf('You can\'t use @AfterSetup on a service.')); + $definitions[$service['id']] = $definition; } - $definitions[$classMetadata->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..6cf5bc8 --- /dev/null +++ b/Tests/Functional/Bundle/TestBundle/Service/Multi.php @@ -0,0 +1,49 @@ +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; + } +} diff --git a/Tests/Functional/MultiServiceInjectTest.php b/Tests/Functional/MultiServiceInjectTest.php new file mode 100644 index 0000000..9fb4b72 --- /dev/null +++ b/Tests/Functional/MultiServiceInjectTest.php @@ -0,0 +1,45 @@ +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($nice, $second->nice, 'all injections without restriction should be done'); + $this->assertSame($bar, $second->worker); + + $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/Driver/AnnotationDriverTest.php b/Tests/Metadata/Driver/AnnotationDriverTest.php index c6814af..0b0e5d5 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 { @@ -43,6 +44,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 @@ +