From f2deb1602a030e75f259164552aa1644fad0f0ce Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Sun, 5 Apr 2020 22:44:43 +0200
Subject: [PATCH] [TASK] Remove internal Extbase Object Container code
Since the ObjectManagerInterface methods.
- isRegistered()
- getScope()
are internal, they are substituted by PHP's internal code, and avoid
look ups to the custom ObjectContainer code.
Two constants and further methods about Singleton checks
within Extbase's internal ObjectContainer are removed as they
are now unused as well.
Resolves: #91002
Releases: master
Change-Id: I1ed9ae2749aeabb6ab352057e639946289dde96a
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64083
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
---
.../Classes/Object/Container/Container.php | 53 +++----------------
.../extbase/Classes/Object/ObjectManager.php | 27 ----------
.../Classes/Object/ObjectManagerInterface.php | 18 -------
.../extbase/Classes/SignalSlot/Dispatcher.php | 2 +-
.../Classes/Validation/ValidatorResolver.php | 13 ++---
.../Unit/Object/Container/ContainerTest.php | 20 +------
.../SignalSlot/DispatcherTest.php | 3 --
7 files changed, 14 insertions(+), 122 deletions(-)
diff --git a/typo3/sysext/extbase/Classes/Object/Container/Container.php b/typo3/sysext/extbase/Classes/Object/Container/Container.php
index 618930d2ea37..e383f109e162 100644
--- a/typo3/sysext/extbase/Classes/Object/Container/Container.php
+++ b/typo3/sysext/extbase/Classes/Object/Container/Container.php
@@ -20,7 +20,6 @@ use Doctrine\Instantiator\InstantiatorInterface;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;
-use Psr\Log\LoggerInterface;
use TYPO3\CMS\Core\SingletonInterface;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Extbase\Reflection\ClassSchema;
@@ -34,9 +33,6 @@ class Container implements SingletonInterface, LoggerAwareInterface
{
use LoggerAwareTrait;
- const SCOPE_PROTOTYPE = 1;
- const SCOPE_SINGLETON = 2;
-
/**
* @var ContainerInterface
*/
@@ -189,7 +185,7 @@ class Container implements SingletonInterface, LoggerAwareInterface
if ($classIsSingleton && !empty($givenConstructorArguments)) {
throw new \TYPO3\CMS\Extbase\Object\Exception('Object "' . $className . '" has explicit constructor arguments but is a singleton; this is not allowed.', 1292858051);
}
- $constructorArguments = $this->getConstructorArguments($className, $classSchema, $givenConstructorArguments);
+ $constructorArguments = $this->getConstructorArguments($classSchema, $givenConstructorArguments);
$instance = GeneralUtility::makeInstance($className, ...$constructorArguments);
if ($classIsSingleton) {
$this->singletonInstances[$className] = $instance;
@@ -210,8 +206,8 @@ class Container implements SingletonInterface, LoggerAwareInterface
}
foreach ($classSchema->getInjectMethods() as $injectMethodName => $injectMethod) {
$instanceToInject = $this->getInstanceInternal($injectMethod->getFirstParameter()->getDependency());
- if ($classSchema->isSingleton() && !$instanceToInject instanceof \TYPO3\CMS\Core\SingletonInterface) {
- $this->getLogger()->notice('The singleton "' . $classSchema->getClassName() . '" needs a prototype in "' . $injectMethodName . '". This is often a bad code smell; often you rather want to inject a singleton.');
+ if ($classSchema->isSingleton() && !$instanceToInject instanceof SingletonInterface) {
+ $this->logger->notice('The singleton "' . $classSchema->getClassName() . '" needs a prototype in "' . $injectMethodName . '". This is often a bad code smell; often you rather want to inject a singleton.');
}
if (is_callable([$instance, $injectMethodName])) {
$instance->{$injectMethodName}($instanceToInject);
@@ -221,8 +217,8 @@ class Container implements SingletonInterface, LoggerAwareInterface
$classNameToInject = $injectProperty->getType();
$instanceToInject = $this->getInstanceInternal($classNameToInject);
- if ($classSchema->isSingleton() && !$instanceToInject instanceof \TYPO3\CMS\Core\SingletonInterface) {
- $this->getLogger()->notice('The singleton "' . $classSchema->getClassName() . '" needs a prototype in "' . $injectPropertyName . '". This is often a bad code smell; often you rather want to inject a singleton.');
+ if ($classSchema->isSingleton() && !$instanceToInject instanceof SingletonInterface) {
+ $this->logger->notice('The singleton "' . $classSchema->getClassName() . '" needs a prototype in "' . $injectPropertyName . '". This is often a bad code smell; often you rather want to inject a singleton.');
}
if ($classSchema->getProperty($injectPropertyName)->isPublic()) {
@@ -263,18 +259,13 @@ class Container implements SingletonInterface, LoggerAwareInterface
/**
* gets array of parameter that can be used to call a constructor
*
- * @param string $className
* @param ClassSchema $classSchema
* @param array $givenConstructorArguments
* @throws \InvalidArgumentException
* @return array
*/
- private function getConstructorArguments(string $className, ClassSchema $classSchema, array $givenConstructorArguments): array
+ private function getConstructorArguments(ClassSchema $classSchema, array $givenConstructorArguments): array
{
- // @todo: drop the $className argument here.
- // @todo: 1) we have that via the $classSchema object
- // @todo: 2) if we drop the log message, the argument is superfluous
- //
// @todo: -> private function getConstructorArguments(Method $constructor, array $givenConstructorArguments)
if (!$classSchema->hasConstructor()) {
@@ -295,8 +286,8 @@ class Container implements SingletonInterface, LoggerAwareInterface
} else {
if ($methodParameter->getDependency() !== null && !$methodParameter->hasDefaultValue()) {
$argument = $this->getInstanceInternal($methodParameter->getDependency());
- if ($classSchema->isSingleton() && !$argument instanceof \TYPO3\CMS\Core\SingletonInterface) {
- $this->getLogger()->notice('The singleton "' . $className . '" needs a prototype in the constructor. This is often a bad code smell; often you rather want to inject a singleton.');
+ if ($classSchema->isSingleton() && !$argument instanceof SingletonInterface) {
+ $this->logger->notice('The singleton "' . $classSchema->getClassName() . '" needs a prototype in the constructor. This is often a bad code smell; often you rather want to inject a singleton.');
// todo: the whole injection is flawed anyway, why would we care about injecting prototypes? so, wayne?
// todo: btw: if this is important, we can already detect this case in the class schema.
}
@@ -329,34 +320,6 @@ class Container implements SingletonInterface, LoggerAwareInterface
return $className;
}
- /**
- * @param string $className
- *
- * @return bool
- */
- public function isSingleton(string $className): bool
- {
- return in_array(SingletonInterface::class, class_implements($className, true), true);
- }
-
- /**
- * @param string $className
- *
- * @return bool
- */
- public function isPrototype(string $className): bool
- {
- return !$this->isSingleton($className);
- }
-
- /**
- * @return LoggerInterface
- */
- protected function getLogger(): LoggerInterface
- {
- return $this->logger;
- }
-
/**
* Lazy load ReflectionService.
*
diff --git a/typo3/sysext/extbase/Classes/Object/ObjectManager.php b/typo3/sysext/extbase/Classes/Object/ObjectManager.php
index f576e9e8b4ab..7ca72630fc83 100644
--- a/typo3/sysext/extbase/Classes/Object/ObjectManager.php
+++ b/typo3/sysext/extbase/Classes/Object/ObjectManager.php
@@ -87,18 +87,6 @@ class ObjectManager implements ObjectManagerInterface
);
}
- /**
- * Returns TRUE if an object with the given name is registered
- *
- * @param string $objectName Name of the object
- * @return bool TRUE if the object has been registered, otherwise FALSE
- * @internal only to be used within Extbase, not part of TYPO3 Core API.
- */
- public function isRegistered(string $objectName): bool
- {
- return class_exists($objectName, true);
- }
-
/**
* Returns a fresh or existing instance of the object specified by $objectName.
*
@@ -126,21 +114,6 @@ class ObjectManager implements ObjectManagerInterface
return $this->objectContainer->getInstance($objectName, $constructorArguments);
}
- /**
- * Returns the scope of the specified object.
- *
- * @param string $objectName The object name
- * @return int One of the ExtbaseContainer::SCOPE_ constants
- * @throws \TYPO3\CMS\Extbase\Object\Container\Exception\UnknownObjectException
- */
- public function getScope(string $objectName): int
- {
- if (!$this->isRegistered($objectName)) {
- throw new \TYPO3\CMS\Extbase\Object\Container\Exception\UnknownObjectException('Object "' . $objectName . '" is not registered.', 1265367590);
- }
- return $this->objectContainer->isSingleton($objectName) ? ExtbaseContainer::SCOPE_SINGLETON : ExtbaseContainer::SCOPE_PROTOTYPE;
- }
-
/**
* Create an instance of $className without calling its constructor
*
diff --git a/typo3/sysext/extbase/Classes/Object/ObjectManagerInterface.php b/typo3/sysext/extbase/Classes/Object/ObjectManagerInterface.php
index 12eaa7ff3b24..80240cb082f5 100644
--- a/typo3/sysext/extbase/Classes/Object/ObjectManagerInterface.php
+++ b/typo3/sysext/extbase/Classes/Object/ObjectManagerInterface.php
@@ -21,15 +21,6 @@ namespace TYPO3\CMS\Extbase\Object;
*/
interface ObjectManagerInterface extends \TYPO3\CMS\Core\SingletonInterface
{
- /**
- * Returns TRUE if an object with the given name is registered
- *
- * @param string $objectName Name of the object
- * @return bool TRUE if the object has been registered, otherwise FALSE
- * @internal only to be used within Extbase, not part of TYPO3 Core API.
- */
- public function isRegistered(string $objectName): bool;
-
/**
* Returns a fresh or existing instance of the object specified by $objectName.
*
@@ -46,13 +37,4 @@ interface ObjectManagerInterface extends \TYPO3\CMS\Core\SingletonInterface
* @return object
*/
public function getEmptyObject(string $className): object;
-
- /**
- * Returns the scope of the specified object.
- *
- * @param string $objectName The object name
- * @return int One of the Container::SCOPE_ constants
- * @internal only to be used within Extbase, not part of TYPO3 Core API.
- */
- public function getScope(string $objectName): int;
}
diff --git a/typo3/sysext/extbase/Classes/SignalSlot/Dispatcher.php b/typo3/sysext/extbase/Classes/SignalSlot/Dispatcher.php
index cca6451186f8..4073e702a5f8 100644
--- a/typo3/sysext/extbase/Classes/SignalSlot/Dispatcher.php
+++ b/typo3/sysext/extbase/Classes/SignalSlot/Dispatcher.php
@@ -360,7 +360,7 @@ class Dispatcher implements \TYPO3\CMS\Core\SingletonInterface
if (isset($slotInformation['object'])) {
$object = $slotInformation['object'];
} else {
- if (!$this->objectManager->isRegistered($slotInformation['class'])) {
+ if (!class_exists($slotInformation['class'])) {
throw new Exception\InvalidSlotException('The given class "' . $slotInformation['class'] . '" is not a registered object.', 1245673367);
}
$object = $this->objectManager->get($slotInformation['class']);
diff --git a/typo3/sysext/extbase/Classes/Validation/ValidatorResolver.php b/typo3/sysext/extbase/Classes/Validation/ValidatorResolver.php
index acc122444aa8..e2440951f1d8 100644
--- a/typo3/sysext/extbase/Classes/Validation/ValidatorResolver.php
+++ b/typo3/sysext/extbase/Classes/Validation/ValidatorResolver.php
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Extbase\Validation;
*/
use TYPO3\CMS\Core\Log\LogManager;
+use TYPO3\CMS\Core\SingletonInterface;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Extbase\Utility\TypeHandlingUtility;
use TYPO3\CMS\Extbase\Validation\Exception\NoSuchValidatorException;
@@ -165,7 +166,7 @@ class ValidatorResolver implements \TYPO3\CMS\Core\SingletonInterface
]
);
$objectValidator->addPropertyValidator($property->getName(), $collectionValidator);
- } elseif (class_exists($propertyTargetClassName) && !TypeHandlingUtility::isCoreType($propertyTargetClassName) && $this->objectManager->isRegistered($propertyTargetClassName) && $this->objectManager->getScope($propertyTargetClassName) === \TYPO3\CMS\Extbase\Object\Container\Container::SCOPE_PROTOTYPE) {
+ } elseif (class_exists($propertyTargetClassName) && !TypeHandlingUtility::isCoreType($propertyTargetClassName) && !in_array(SingletonInterface::class, class_implements($propertyTargetClassName, true), true)) {
/*
* class_exists($propertyTargetClassName) checks, if the type of the property is an object
* instead of a simple type. Like DateTime or another model.
@@ -174,14 +175,8 @@ class ValidatorResolver implements \TYPO3\CMS\Core\SingletonInterface
* is not a core type, which are Enums and File objects for example.
* todo: check why these types should not be validated
*
- * $this->objectManager->isRegistered($propertyTargetClassName) checks if the type is a class
- * which can be loaded.
- * todo: this could be dropped as it's the same check as the first one.
- *
- * $this->objectManager->getScope($propertyTargetClassName) ===
- * \TYPO3\CMS\Extbase\Object\Container\Container::SCOPE_PROTOTYPE checks, if the type of the
- * property is a prototype, meaning that it's a class which does not implement the
- * SingletonInterface
+ * !in_array(SingletonInterface::class, class_implements($propertyTargetClassName, true), true)
+ * checks if the class is an instance of a Singleton
* todo: check why Singletons shouldn't be validated.
*/
diff --git a/typo3/sysext/extbase/Tests/Unit/Object/Container/ContainerTest.php b/typo3/sysext/extbase/Tests/Unit/Object/Container/ContainerTest.php
index 4dd274bfb846..27e8ed705dcb 100644
--- a/typo3/sysext/extbase/Tests/Unit/Object/Container/ContainerTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Object/Container/ContainerTest.php
@@ -66,7 +66,7 @@ class ContainerTest extends UnitTestCase
->setConstructorArgs([$psrContainer])
->setMethods(['getLogger', 'getReflectionService'])
->getMock();
- $this->subject->expects(self::any())->method('getLogger')->willReturn($this->logger);
+ $this->subject->setLogger($this->logger);
$this->subject->expects(self::any())->method('getReflectionService')->willReturn($reflectionService);
}
@@ -357,24 +357,6 @@ class ContainerTest extends UnitTestCase
self::assertInstanceOf('t3lib_object_singleton', $object->dependency);
}
- /**
- * @test
- */
- public function isSingletonReturnsTrueForSingletonInstancesAndFalseForPrototypes()
- {
- self::assertTrue($this->subject->isSingleton(Container::class));
- self::assertFalse($this->subject->isSingleton(\TYPO3\CMS\Extbase\Core\Bootstrap::class));
- }
-
- /**
- * @test
- */
- public function isPrototypeReturnsFalseForSingletonInstancesAndTrueForPrototypes()
- {
- self::assertFalse($this->subject->isPrototype(Container::class));
- self::assertTrue($this->subject->isPrototype(\TYPO3\CMS\Extbase\Core\Bootstrap::class));
- }
-
/************************************************
* Test regarding constructor argument injection
************************************************/
diff --git a/typo3/sysext/extbase/Tests/UnitDeprecated/SignalSlot/DispatcherTest.php b/typo3/sysext/extbase/Tests/UnitDeprecated/SignalSlot/DispatcherTest.php
index d0e67bbbfaf5..f00c83ca6abf 100644
--- a/typo3/sysext/extbase/Tests/UnitDeprecated/SignalSlot/DispatcherTest.php
+++ b/typo3/sysext/extbase/Tests/UnitDeprecated/SignalSlot/DispatcherTest.php
@@ -131,7 +131,6 @@ class DispatcherTest extends UnitTestCase
{
$slotClassName = OnlyClassNameSpecifiedFixture::class;
$mockSlot = new OnlyClassNameSpecifiedFixture();
- $this->objectManagerProphecy->isRegistered($slotClassName)->willReturn(true);
$this->objectManagerProphecy->get($slotClassName)->willReturn($mockSlot);
$this->signalSlotDispatcher->connect('Foo', 'emitBar', $slotClassName, 'slot', false);
$this->signalSlotDispatcher->dispatch('Foo', 'emitBar', ['bar', 'quux']);
@@ -267,7 +266,6 @@ class DispatcherTest extends UnitTestCase
{
$this->expectException(InvalidSlotException::class);
$this->expectExceptionCode(1245673367);
- $this->objectManagerProphecy->isRegistered('NonExistingClassName')->willReturn(false);
$this->signalSlotDispatcher->connect('Foo', 'emitBar', 'NonExistingClassName', 'slot', true);
$this->signalSlotDispatcher->dispatch('Foo', 'emitBar', []);
}
@@ -281,7 +279,6 @@ class DispatcherTest extends UnitTestCase
$this->expectExceptionCode(1245673368);
$slotClassName = SlotMethodDoesNotExistFixture::class;
$mockSlot = new SlotMethodDoesNotExistFixture();
- $this->objectManagerProphecy->isRegistered($slotClassName)->willReturn(true);
$this->objectManagerProphecy->get($slotClassName)->willReturn($mockSlot);
$this->signalSlotDispatcher->connect('Foo', 'emitBar', $slotClassName, 'unknownMethodName', true);
$this->signalSlotDispatcher->dispatch('Foo', 'emitBar', ['bar', 'quux']);
--
GitLab