Merge pull request #35 from Ezaki113/master

Fix bug "Validation fails with factory service "public" set to `false`"

Author: matthiasnoback

ConstructorResolver.php

@@ -36,6 +36,10 @@ public function resolve(Definition $definition)
             return $this->resolveFactoryServiceWithMethod((string) $factory[0], $factory[1]);
         }
 
+        if (is_array($factory) && $factory[0] instanceof Definition) {
+            return $this->resolveFactoryServiceDefinitionWithMethod($factory[0], $factory[1]);
+        }
+
         if (is_array($factory)) {
             return $this->resolveFactoryClassWithMethod($factory[0], $factory[1]);
         }
@@ -133,4 +137,15 @@ private function resolveFactoryFunction($factory)
 
         return new \ReflectionFunction($factory);
     }
+
+    private function resolveFactoryServiceDefinitionWithMethod(Definition $factoryDefinition, $factoryMethod)
+    {
+        $factoryClass = $this->resultingClassResolver->resolve($factoryDefinition);
+
+        if (!method_exists($factoryClass, $factoryMethod)) {
+            throw new MethodNotFoundException($factoryClass, $factoryMethod);
+        }
+
+        return new \ReflectionMethod($factoryClass, $factoryMethod);
+    }
 }

ServiceDefinitionValidator.php

@@ -9,7 +9,6 @@
 use Matthias\SymfonyServiceDefinitionValidator\Exception\ServiceNotFoundException;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
-use Symfony\Component\DependencyInjection\Reference;
 
 class ServiceDefinitionValidator implements ServiceDefinitionValidatorInterface
 {
@@ -95,6 +94,8 @@ private function validateFactory(Definition $definition)
             list($factoryClassOrService, $method) = $factory;
             if (is_string($factoryClassOrService)) {
                 $this->validateFactoryClassAndMethod($factoryClassOrService, $method);
+            } elseif ($factoryClassOrService instanceof Definition) {
+                $this->validateFactoryServiceDefinitionAndMethod($factoryClassOrService, $method);
             } else {
                 $this->validateFactoryServiceAndMethod((string) $factoryClassOrService, $method);
             }
@@ -164,4 +165,15 @@ private function validateFactoryServiceAndMethod($factoryServiceId, $factoryMeth
 
         $this->validateFactoryClassAndMethod($factoryClass, $factoryMethod);
     }
+
+    private function validateFactoryServiceDefinitionAndMethod(Definition $factoryServiceDefinition, $factoryMethod)
+    {
+        if (!$factoryMethod) {
+            throw new MissingFactoryMethodException();
+        }
+
+        $factoryClass = $factoryServiceDefinition->getClass();
+
+        $this->validateFactoryClassAndMethod($factoryClass, $factoryMethod);
+    }
 }

Tests/Functional/Fixtures/reported_problems.yml

@@ -7,3 +7,14 @@ services:
             password: "password"
         calls:
             - [setAttribute, [3, 2]]
+
+    # Issue 33: https://github.com/matthiasnoback/symfony-service-definition-validator/issues/33
+    service_created_by_nonpublic_factory:
+        class: stdClass
+        factory: ["@nonpublic_factory", "create"]
+        arguments:
+            - required_argument
+
+    nonpublic_factory:
+        class: Matthias\SymfonyServiceDefinitionValidator\Tests\Functional\Fixtures\Factory
+        public: false

Tests/ServiceDefinitionValidatorTest.php

@@ -228,6 +228,102 @@ public function ifFactoryMethodDoesNotExistOnFactoryServiceFails()
         $validator->validate($definition);
     }
 
+    /**
+     * @test
+     */
+    public function factoryCanBeProvidedByServiceDefinition()
+    {
+        if (!method_exists('Symfony\Component\DependencyInjection\Definition', 'getFactory')) {
+            $this->markTestSkipped('Factory can be provided by service definition since Symfony 2.6');
+        }
+
+        $factoryDefinition = new Definition('Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\FactoryClass');
+        $definition = new Definition('stdClass');
+        $definition->setFactory(array($factoryDefinition, 'create'));
+
+        $containerBuilder = new ContainerBuilder();
+        $validator = new ServiceDefinitionValidator(
+            $containerBuilder,
+            $this->createMockDefinitionArgumentsValidator(),
+            $this->createMockMethodCallsValidator()
+        );
+
+        $validator->validate($definition);
+    }
+
+    /**
+     * @test
+     */
+    public function ifFactoryProvidedByServiceDefinitionClassDoesNotExistFails()
+    {
+        if (!method_exists('Symfony\Component\DependencyInjection\Definition', 'getFactory')) {
+            $this->markTestSkipped('Factory can be provided by service definition since Symfony 2.6');
+        }
+
+        $factoryDefinition = new Definition('Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\MissingFactoryClass');
+        $definition = new Definition('stdClass');
+        $definition->setFactory(array($factoryDefinition, 'create'));
+
+        $containerBuilder = new ContainerBuilder();
+        $validator = new ServiceDefinitionValidator(
+            $containerBuilder,
+            $this->createMockDefinitionArgumentsValidator(),
+            $this->createMockMethodCallsValidator()
+        );
+
+        $this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\ClassNotFoundException');
+        $validator->validate($definition);
+    }
+
+    /**
+     * @test
+     */
+    public function ifFactoryProvidedByServiceDefinitionSpecifiedWithoutFactoryMethodFails()
+    {
+        if (!method_exists('Symfony\Component\DependencyInjection\Definition', 'getFactory')) {
+            $this->markTestSkipped('Factory can be provided by service definition since Symfony 2.6');
+        }
+
+        $factoryDefinition = new Definition('Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\FactoryClass');
+        $definition = new Definition('stdClass');
+        $definition->setFactory(array($factoryDefinition, ''));
+
+        $containerBuilder = new ContainerBuilder();
+        $validator = new ServiceDefinitionValidator(
+            $containerBuilder,
+            $this->createMockDefinitionArgumentsValidator(),
+            $this->createMockMethodCallsValidator()
+        );
+
+        $this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\MissingFactoryMethodException');
+        $validator->validate($definition);
+    }
+
+    /**
+     * @test
+     */
+    public function ifFactoryMethodDoesNotExistOnFactoryServiceProvidedByDefinitionFails()
+    {
+        if (!method_exists('Symfony\Component\DependencyInjection\Definition', 'getFactory')) {
+            $this->markTestSkipped('Factory can be provided by service definition since Symfony 2.6');
+        }
+
+        $factoryDefinition = new Definition('Matthias\SymfonyServiceDefinitionValidator\Tests\Fixtures\FactoryClass');
+        $definition = new Definition('stdClass');
+        $definition->setFactory(array($factoryDefinition, 'nonExistingFactoryMethod'));
+
+        $containerBuilder = new ContainerBuilder();
+        $validator = new ServiceDefinitionValidator(
+            $containerBuilder,
+            $this->createMockDefinitionArgumentsValidator(),
+            $this->createMockMethodCallsValidator()
+        );
+
+        $this->setExpectedException('Matthias\SymfonyServiceDefinitionValidator\Exception\MethodNotFoundException');
+        $validator->validate($definition);
+    }
+
+
     private function getNonExistingClassName()
     {
         return md5(rand(1, 999));