[BUGFIX] Extbase (Object): Allow injection of prototypes via dependency injection
authorSebastian Kurfürst <sebastian@typo3.org>
Fri, 21 Jan 2011 12:34:03 +0000 (12:34 +0000)
committerSebastian Kurfürst <sebastian@typo3.org>
Fri, 21 Jan 2011 12:34:03 +0000 (12:34 +0000)
This change allows Injection of Prototypes into singletons
and prototypes. Now, all four cases are supported:

* Singletons which need Singletons (most common)
* Singletons which need Prototypes (very uncommon,
  as it usually hints at the fact that the injected
  prototype should be a singleton). In this case,
  WE WRITE A DEVLOG MESSAGE
* Prototypes which need Singletons
* Prototypes which need Prototypes

All cases above are covered with unit tests, both
when using setter injection and when using constructor
injection.

Additionally, I greatly improved the unit tests, and added
working cycle detection in case of prototype objects
depending on each other.

Resolves: #12013

typo3/sysext/extbase/Classes/Object/Container/Container.php
typo3/sysext/extbase/Classes/Object/Exception/CannotBuildObject.php
typo3/sysext/extbase/Tests/Unit/Object/Container/ContainerTest.php
typo3/sysext/extbase/Tests/Unit/Object/Container/Fixtures/Testclasses.php

index 7dead80..596e874 100644 (file)
@@ -63,6 +63,13 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
        private $singletonInstances = array();
 
        /**
+        * Array of prototype objects currently being built, to prevent recursion.
+        *
+        * @var array
+        */
+       private $prototypeObjectsWhichAreCurrentlyInstanciated;
+
+       /**
         * Constructor is protected since container should
         * be a singleton.
         *
@@ -84,6 +91,18 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
         * @return object the built object
         */
        public function getInstance($className, $givenConstructorArguments = array()) {
+               $this->prototypeObjectsWhichAreCurrentlyInstanciated = array();
+               return $this->getInstanceInternal($className, $givenConstructorArguments);
+       }
+
+       /**
+        * Internal implementation for getting a class.
+        *
+        * @param string $className
+        * @param array $givenConstructorArguments the list of constructor arguments as array
+        * @return object the built object
+        */
+       protected function getInstanceInternal($className, $givenConstructorArguments = array()) {
                $className = $this->getImplementationClassName($className);
 
                if ($className === 'Tx_Extbase_Object_Container_Container') {
@@ -96,6 +115,15 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
                        }
                        return $this->singletonInstances[$className];
                }
+
+               $classIsSingleton = $this->isSingleton($className);
+               if (!$classIsSingleton) {
+                       if (array_search($className, $this->prototypeObjectsWhichAreCurrentlyInstanciated) !== FALSE) {
+                               throw new Tx_Extbase_Object_Exception_CannotBuildObject('Cyclic dependency in prototype object, for class "' . $className . '".', 1295611406);
+                       }
+                       $this->prototypeObjectsWhichAreCurrentlyInstanciated[] = $className;
+               }
+
                $classInfo = $this->getClassInfo($className);
 
                $instance = $this->instanciateObject($className, $classInfo, $givenConstructorArguments);
@@ -105,6 +133,10 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
                        $instance->initializeObject();
                }
 
+               if (!$classIsSingleton) {
+                       array_pop($this->prototypeObjectsWhichAreCurrentlyInstanciated);
+               }
+
                return $instance;
        }
 
@@ -125,7 +157,7 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
                        throw new Tx_Extbase_Object_Exception('Object "' . $className . '" has explicit constructor arguments but is a singleton; this is not allowed.', 1292858051);
                }
 
-               $constructorArguments = $this->getConstructorArguments($classInfo->getConstructorArguments(), $givenConstructorArguments, $level);
+               $constructorArguments = $this->getConstructorArguments($className, $classInfo->getConstructorArguments(), $givenConstructorArguments);
                array_unshift($constructorArguments, $className);
                $instance = call_user_func_array(array('t3lib_div', 'makeInstance'), $constructorArguments);
 
@@ -147,15 +179,27 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
 
                foreach ($classInfo->getInjectMethods() as $injectMethodName => $classNameToInject) {
 
-                       $instanceToInject = $this->getInstance($classNameToInject);
-                       if (!$instanceToInject instanceof t3lib_Singleton) {
-                               throw new Tx_Extbase_Object_Exception_WrongScope('Setter dependencies can only be singletons', 1292860859);
+                       $instanceToInject = $this->getInstanceInternal($classNameToInject);
+                       if ($this->isSingleton($instance) && !($instanceToInject instanceof t3lib_Singleton)) {
+                               $this->log('The singleton "' . $classInfo->getClassName() . '" needs a prototype in "' . $injectMethodName . '". This is often a bad code smell; often you rather want to inject a singleton.', 1);
                        }
+
                        $instance->$injectMethodName($instanceToInject);
                }
        }
 
        /**
+        * Wrapper for dev log, in order to ease testing
+        *
+        * @param       string          Message (in english).
+        * @param       integer         Severity: 0 is info, 1 is notice, 2 is warning, 3 is fatal error, -1 is "OK" message
+        * @return      void
+        */
+       protected function log($message, $severity) {
+               t3lib_div::devLog($message, 'extbase', $severity);
+       }
+
+       /**
         * register a classname that should be used if a dependency is required.
         * e.g. used to define default class for a interface
         *
@@ -169,11 +213,12 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
        /**
         * gets array of parameter that can be used to call a constructor
         *
+        * @param string $className
         * @param array $constructorArgumentInformation
         * @param array $givenConstructorArguments
         * @return array
         */
-       private function getConstructorArguments(array $constructorArgumentInformation, array $givenConstructorArguments, $level) {
+       private function getConstructorArguments($className, array $constructorArgumentInformation, array $givenConstructorArguments) {
                $parameters=array();
                foreach ($constructorArgumentInformation as $argumentInformation) {
                        $argumentName = $argumentInformation['name'];
@@ -182,10 +227,10 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
                        // AND the class has NOT been explicitely passed in
                        if (isset($argumentInformation['dependency']) && !(count($givenConstructorArguments) && is_a($givenConstructorArguments[0], $argumentInformation['dependency']))) {
                                // Inject parameter
-                               if (!$this->isSingleton($argumentInformation['dependency'])) {
-                                       throw new Tx_Extbase_Object_Exception_WrongScope('Constructor dependencies can only be singletons', 1292860858);
+                               $parameter = $this->getInstanceInternal($argumentInformation['dependency']);
+                               if ($this->isSingleton($className) && !($parameter instanceof t3lib_Singleton)) {
+                                       $this->log('The singleton "' . $className . '" needs a prototype in the constructor. This is often a bad code smell; often you rather want to inject a singleton.', 1);
                                }
-                               $parameter = $this->getInstance($argumentInformation['dependency']);
                        } elseif (count($givenConstructorArguments)) {
                                // EITHER:
                                // No dependency injectable anymore, but we still have
@@ -240,7 +285,7 @@ class Tx_Extbase_Object_Container_Container implements t3lib_Singleton {
         */
        private function getClassInfo($className) {
                        // we also need to make sure that the cache is returning a vaild object
-                       // in case something went wrong with unserialization etc.. 
+                       // in case something went wrong with unserialization etc..
                if (!$this->cache->has($className) || !is_object($this->cache->get($className))) {
                        $this->cache->set($className, $this->classInfoFactory->buildClassInfoFromClassName($className));
                }
index 5e797df..ffff0c7 100644 (file)
@@ -32,7 +32,7 @@
  * @subpackage Object\Exception
  * @version $Id$
  */
-class Tx_Extbase_Object_CannotBuildObject extends Tx_Extbase_Object_Exception {
+class Tx_Extbase_Object_Exception_CannotBuildObject extends Tx_Extbase_Object_Exception {
 
 }
 
index 90663db..658fe0e 100644 (file)
@@ -37,8 +37,7 @@ class Tx_Extbase_Tests_Unit_Object_Container_ContainerTest extends Tx_Extbase_Te
        private $container;
 
        public function setUp() {
-               $this->container = new Tx_Extbase_Object_Container_Container();
-
+               $this->container = $this->getMock('Tx_Extbase_Object_Container_Container', array('log'));
        }
 
        /**
@@ -127,18 +126,17 @@ class Tx_Extbase_Tests_Unit_Object_Container_ContainerTest extends Tx_Extbase_Te
 
        /**
         * @test
-        * @expectedException Tx_Extbase_Object_Exception
+        * @expectedException Tx_Extbase_Object_Exception_CannotBuildObject
         */
-       public function getInstanceThrowsExceptionIfObjectContainsCyclicDependencyAndIsNoSingleton() {
-               $this->container->getInstance('t3lib_object_tests_cyclic1');
+       public function getInstanceThrowsExceptionIfPrototypeObjectsWiredViaConstructorInjectionContainCyclicDependencies() {
+               $this->container->getInstance('t3lib_object_tests_cyclic1WithSetterDependency');
        }
 
-
        /**
         * @test
-        * @expectedException Tx_Extbase_Object_Exception
+        * @expectedException Tx_Extbase_Object_Exception_CannotBuildObject
         */
-       public function getInstanceThrowsExceptionIfObjectContainsCyclicDependency() {
+       public function getInstanceThrowsExceptionIfPrototypeObjectsWiredViaSetterInjectionContainCyclicDependencies() {
                $this->container->getInstance('t3lib_object_tests_cyclic1');
        }
 
@@ -148,7 +146,6 @@ class Tx_Extbase_Tests_Unit_Object_Container_ContainerTest extends Tx_Extbase_Te
         */
        public function getInstanceThrowsExceptionIfClassWasNotFound() {
                $this->container->getInstance('nonextistingclass_bla');
-
        }
 
        /**
@@ -166,6 +163,9 @@ class Tx_Extbase_Tests_Unit_Object_Container_ContainerTest extends Tx_Extbase_Te
                $this->container->registerImplementation('t3lib_object_tests_someinterface', 't3lib_object_tests_someimplementation');
                $object = $this->container->getInstance('t3lib_object_tests_needsinterface');
                $this->assertType('t3lib_object_tests_needsinterface', $object);
+
+               $this->assertType('t3lib_object_tests_someinterface', $object->dependency);
+               $this->assertType('t3lib_object_tests_someimplementation', $object->dependency);
        }
 
        /**
@@ -177,9 +177,84 @@ class Tx_Extbase_Tests_Unit_Object_Container_ContainerTest extends Tx_Extbase_Te
                $this->assertType('t3lib_object_tests_resolveablecyclic1', $object->o2->o3->o1);
        }
 
+       /**
+        * @test
+        */
+       public function singletonWhichRequiresPrototypeViaSetterInjectionWorksAndAddsDebugMessage() {
+               $this->container->expects($this->once())->method('log')->with('The singleton "t3lib_object_singletonNeedsPrototype" needs a prototype in "injectDependency". This is often a bad code smell; often you rather want to inject a singleton.', 1);
 
+               $object = $this->container->getInstance('t3lib_object_singletonNeedsPrototype');
+               $this->assertType('t3lib_object_prototype', $object->dependency);
+       }
 
-}
+       /**
+        * @test
+        */
+       public function singletonWhichRequiresSingletonViaSetterInjectionWorks() {
+               $this->container->expects($this->never())->method('log');
+
+               $object = $this->container->getInstance('t3lib_object_singletonNeedsSingleton');
+               $this->assertType('t3lib_object_singleton', $object->dependency);
+       }
+
+       /**
+        * @test
+        */
+       public function prototypeWhichRequiresPrototypeViaSetterInjectionWorks() {
+               $this->container->expects($this->never())->method('log');
+
+               $object = $this->container->getInstance('t3lib_object_prototypeNeedsPrototype');
+               $this->assertType('t3lib_object_prototype', $object->dependency);
+       }
+
+       /**
+        * @test
+        */
+       public function prototypeWhichRequiresSingletonViaSetterInjectionWorks() {
+               $this->container->expects($this->never())->method('log');
+
+               $object = $this->container->getInstance('t3lib_object_prototypeNeedsSingleton');
+               $this->assertType('t3lib_object_singleton', $object->dependency);
+       }
+
+       /**
+        * @test
+        */
+       public function singletonWhichRequiresPrototypeViaConstructorInjectionWorksAndAddsDebugMessage() {
+               $this->container->expects($this->once())->method('log')->with('The singleton "t3lib_object_singletonNeedsPrototypeInConstructor" needs a prototype in the constructor. This is often a bad code smell; often you rather want to inject a singleton.', 1);
+
+               $object = $this->container->getInstance('t3lib_object_singletonNeedsPrototypeInConstructor');
+               $this->assertType('t3lib_object_prototype', $object->dependency);
+       }
+
+       /**
+        * @test
+        */
+       public function singletonWhichRequiresSingletonViaConstructorInjectionWorks() {
+               $this->container->expects($this->never())->method('log');
 
+               $object = $this->container->getInstance('t3lib_object_singletonNeedsSingletonInConstructor');
+               $this->assertType('t3lib_object_singleton', $object->dependency);
+       }
+
+       /**
+        * @test
+        */
+       public function prototypeWhichRequiresPrototypeViaConstructorInjectionWorks() {
+               $this->container->expects($this->never())->method('log');
+
+               $object = $this->container->getInstance('t3lib_object_prototypeNeedsPrototypeInConstructor');
+               $this->assertType('t3lib_object_prototype', $object->dependency);
+       }
 
-?>
+       /**
+        * @test
+        */
+       public function prototypeWhichRequiresSingletonViaConstructorInjectionWorks() {
+               $this->container->expects($this->never())->method('log');
+
+               $object = $this->container->getInstance('t3lib_object_prototypeNeedsSingletonInConstructor');
+               $this->assertType('t3lib_object_singleton', $object->dependency);
+       }
+}
+?>
\ No newline at end of file
index cb741b7..8047866 100644 (file)
@@ -102,12 +102,12 @@ class t3lib_object_tests_b_child_someimplementation extends t3lib_object_tests_b
  */
 class t3lib_object_tests_needsinterface {
        public function __construct(t3lib_object_tests_someinterface $i) {
-
+               $this->dependency = $i;
        }
 }
 
 /**
- * classes that depends on each other (death look)
+ * Prototype classes that depend on each other
  *
  */
 class t3lib_object_tests_cyclic1 {
@@ -122,6 +122,18 @@ class t3lib_object_tests_cyclic2 {
        }
 }
 
+class t3lib_object_tests_cyclic1WithSetterDependency {
+       public function injectFoo(t3lib_object_tests_cyclic2WithSetterDependency $c) {
+
+       }
+}
+
+class t3lib_object_tests_cyclic2WithSetterDependency {
+       public function injectFoo(t3lib_object_tests_cyclic1WithSetterDependency $c) {
+
+       }
+}
+
 /**
  * class which has setter injections defined
  *
@@ -199,3 +211,58 @@ class t3lib_object_tests_class_with_injectsettings {
        }
 }
 
+/*
+ *  a Singleton requires a Prototype for Injection -> allowed, autowiring active, but in development context we write a log message, as it is bad practice and most likely points to some logic error.
+If a Singleton requires a Singleton for Injection -> allowed, autowiring active
+If a Prototype requires a Prototype for Injection -> allowed, autowiring active
+If a Prototype requires a Singleton for Injection -> allowed, autowiring active
+ */
+class t3lib_object_singleton implements t3lib_Singleton {
+}
+
+class t3lib_object_prototype {
+}
+
+class t3lib_object_singletonNeedsPrototype implements t3lib_Singleton {
+       public function injectDependency(t3lib_object_prototype $dependency) {
+               $this->dependency = $dependency;
+       }
+}
+
+class t3lib_object_singletonNeedsSingleton implements t3lib_Singleton {
+       public function injectDependency(t3lib_object_singleton $dependency) {
+               $this->dependency = $dependency;
+       }
+}
+class t3lib_object_prototypeNeedsPrototype {
+       public function injectDependency(t3lib_object_prototype $dependency) {
+               $this->dependency = $dependency;
+       }
+}
+class t3lib_object_prototypeNeedsSingleton {
+       public function injectDependency(t3lib_object_singleton $dependency) {
+               $this->dependency = $dependency;
+       }
+}
+
+class t3lib_object_singletonNeedsPrototypeInConstructor implements t3lib_Singleton {
+       public function __construct(t3lib_object_prototype $dependency) {
+               $this->dependency = $dependency;
+       }
+}
+
+class t3lib_object_singletonNeedsSingletonInConstructor implements t3lib_Singleton {
+       public function __construct(t3lib_object_singleton $dependency) {
+               $this->dependency = $dependency;
+       }
+}
+class t3lib_object_prototypeNeedsPrototypeInConstructor {
+       public function __construct(t3lib_object_prototype $dependency) {
+               $this->dependency = $dependency;
+       }
+}
+class t3lib_object_prototypeNeedsSingletonInConstructor {
+       public function __construct(t3lib_object_singleton $dependency) {
+               $this->dependency = $dependency;
+       }
+}