[BUGFIX] ExtractorRegistry can not handle Extractors with same priority 02/36502/3
authorFabien Udriot <fabien.udriot@ecodev.ch>
Sat, 31 Jan 2015 13:24:11 +0000 (14:24 +0100)
committerPhilipp Gampe <philipp.gampe@typo3.org>
Sat, 31 Jan 2015 23:24:51 +0000 (00:24 +0100)
The ExtractorRegistry::getExtractors() overrides existing
instances with the same execution priority. There is no check
or notification about this. As instance, it could be that you
have a extractor for Local driver and an extractor for a remote
driver with the same execution priority, that is currently not possible.

The change set also keep in sync some part of the code with
the FileRenderRegistry which is very close to the Extractor Registry.

Change-Id: I42fdd3916410536a0b097d96cf833129f5359d72
Releases: master, 6.2
Resolves: #56727
Reviewed-on: http://review.typo3.org/36502
Reviewed-by: Frans Saris <franssaris@gmail.com>
Tested-by: Frans Saris <franssaris@gmail.com>
Reviewed-by: Philipp Gampe <philipp.gampe@typo3.org>
Tested-by: Philipp Gampe <philipp.gampe@typo3.org>
typo3/sysext/core/Classes/Resource/Index/ExtractorRegistry.php
typo3/sysext/core/Tests/Unit/Resource/Index/ExtractorRegistryTest.php [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/Resource/Rendering/RendererRegistryTest.php

index 6bf2bf3..8f239b2 100644 (file)
@@ -14,11 +14,14 @@ namespace TYPO3\CMS\Core\Resource\Index;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\SingletonInterface;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
 /**
  * Registry for MetaData extraction Services
  *
  */
-class ExtractorRegistry implements \TYPO3\CMS\Core\SingletonInterface {
+class ExtractorRegistry implements SingletonInterface {
 
        /**
         * Registered ClassNames
@@ -39,20 +42,20 @@ class ExtractorRegistry implements \TYPO3\CMS\Core\SingletonInterface {
         * @return ExtractorRegistry
         */
        public static function getInstance() {
-               return \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(\TYPO3\CMS\Core\Resource\Index\ExtractorRegistry::class);
+               return GeneralUtility::makeInstance(\TYPO3\CMS\Core\Resource\Index\ExtractorRegistry::class);
        }
 
        /**
         * Allows to register MetaData extraction to the FAL Indexer
         *
         * @param string $className
-        * @throws \RuntimeException
+        * @throws \InvalidArgumentException
         */
        public function registerExtractionService($className) {
                if (!class_exists($className)) {
-                       throw new \RuntimeException('The class "' . $className . '" you are registering is not available');
+                       throw new \InvalidArgumentException('The class "' . $className . '" you are registering is not available', 1422705270);
                } elseif (!in_array(\TYPO3\CMS\Core\Resource\Index\ExtractorInterface::class, class_implements($className))) {
-                       throw new \RuntimeException('The extractor needs to implement the ExtractorInterface');
+                       throw new \InvalidArgumentException('The extractor needs to implement the ExtractorInterface', 1422705271);
                } else {
                        $this->extractors[] = $className;
                }
@@ -66,12 +69,17 @@ class ExtractorRegistry implements \TYPO3\CMS\Core\SingletonInterface {
        public function getExtractors() {
                if ($this->instances === NULL) {
                        $this->instances = array();
-                       foreach ($this->extractors as $className) {
+
+                       $extractors = array_reverse($this->extractors);
+                       foreach ($extractors as $className) {
                                /** @var ExtractorInterface $object */
-                               $object = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance($className);
-                               $this->instances[$object->getExecutionPriority()] = $object;
+                               $object = $this->createExtractorInstance($className);
+                               $this->instances[] = $object;
+                       }
+
+                       if (count($this->instances) > 1) {
+                               usort($this->instances, array($this, 'compareExtractorPriority'));
                        }
-                       krsort($this->instances);
                }
                return $this->instances;
        }
@@ -96,4 +104,29 @@ class ExtractorRegistry implements \TYPO3\CMS\Core\SingletonInterface {
                return $filteredExtractors;
        }
 
+       /**
+        * Compare the priority of two Extractor classes.
+        * Is used for sorting array of Extractor instances by priority.
+        * We want the result to be ordered from high to low so a higher
+        * priority comes before a lower.
+        *
+        * @param ExtractorInterface $extractorA
+        * @param ExtractorInterface $extractorB
+        * @return int -1 a > b, 0 a == b, 1 a < b
+        */
+       protected function compareExtractorPriority(ExtractorInterface $extractorA, ExtractorInterface $extractorB) {
+               return $extractorB->getPriority() - $extractorA->getPriority();
+       }
+
+       /**
+        * Create an instance of a Metadata Extractor
+        *
+        * @param string $className
+        * @return ExtractorInterface
+        */
+       protected function createExtractorInstance($className) {
+               return GeneralUtility::makeInstance($className);
+       }
+
+
 }
diff --git a/typo3/sysext/core/Tests/Unit/Resource/Index/ExtractorRegistryTest.php b/typo3/sysext/core/Tests/Unit/Resource/Index/ExtractorRegistryTest.php
new file mode 100644 (file)
index 0000000..7e3f1f6
--- /dev/null
@@ -0,0 +1,141 @@
+<?php
+namespace TYPO3\CMS\Core\Tests\Unit\Resource\Index;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use TYPO3\CMS\Core\Tests\UnitTestCase;
+
+/**
+ * Class ExtractorRegistryTest
+ */
+class ExtractorRegistryTest extends UnitTestCase {
+
+       /**
+        * @test
+        */
+       public function registeredExtractorClassCanBeRetrieved() {
+               $extractorClass = 'a9f4d5e4ebb4b03547a2a6094e1170ac';
+               $extractorObject = $this->getMock(\TYPO3\CMS\Core\Resource\Index\ExtractorInterface::class, array(), array(), $extractorClass);
+
+               $extractorRegistry = $this->getMockExtractorRegistry(array(array($extractorClass, $extractorObject)));
+
+               $extractorRegistry->registerExtractionService($extractorClass);
+               $this->assertContains($extractorObject, $extractorRegistry->getExtractors(), '', FALSE, FALSE);
+       }
+
+       /**
+        * @test
+        * @expectedException \InvalidArgumentException
+        * @expectedExceptionCode 1422705270
+        */
+       public function registerExtractorClassThrowsExceptionIfClassDoesNotExist() {
+               $className = 'e1f9aa4e1cd3aa7ff05dcdccb117156a';
+               $extractorRegistry = $this->getMockExtractorRegistry();
+               $extractorRegistry->registerExtractionService($className);
+       }
+
+       /**
+        * @test
+        * @expectedException \InvalidArgumentException
+        * @expectedExceptionCode 1422705271
+        */
+       public function registerExtractorClassThrowsExceptionIfClassDoesNotImplementRightInterface() {
+               $className = __CLASS__;
+               $extractorRegistry = $this->getMockExtractorRegistry();
+               $extractorRegistry->registerExtractionService($className);
+       }
+
+       /**
+        * @test
+        */
+       public function registerExtractorClassWithHighestPriorityIsFirstInResult() {
+
+               $extractorClass1 = 'db76010e5c24658c35ea1605cce2391d';
+               $extractorObject1 = $this->getMock(\TYPO3\CMS\Core\Resource\Index\ExtractorInterface::class, array(), array(), $extractorClass1);
+               $extractorObject1->expects($this->any())->method('getPriority')->will($this->returnValue(1));
+
+               $extractorClass2 = 'ad9195e2487eea33c8a2abd5cf33cba4';
+               $extractorObject2 = $this->getMock(\TYPO3\CMS\Core\Resource\Index\ExtractorInterface::class, array(), array(), $extractorClass2);
+               $extractorObject2->expects($this->any())->method('getPriority')->will($this->returnValue(10));
+
+               $extractorClass3 = 'cef9aa4e1cd3aa7ff05dcdccb117156a';
+               $extractorObject3 = $this->getMock(\TYPO3\CMS\Core\Resource\Index\ExtractorInterface::class, array(), array(), $extractorClass3);
+               $extractorObject3->expects($this->any())->method('getPriority')->will($this->returnValue(2));
+
+               $createdExtractorInstances = array(
+                       array($extractorClass1, $extractorObject1),
+                       array($extractorClass2, $extractorObject2),
+                       array($extractorClass3, $extractorObject3),
+               );
+
+               $extractorRegistry = $this->getMockExtractorRegistry($createdExtractorInstances);
+               $extractorRegistry->registerExtractionService($extractorClass1);
+               $extractorRegistry->registerExtractionService($extractorClass2);
+               $extractorRegistry->registerExtractionService($extractorClass3);
+
+               $extractorInstances = $extractorRegistry->getExtractors();
+
+               $this->assertTrue($extractorInstances[0] instanceof $extractorClass2);
+               $this->assertTrue($extractorInstances[1] instanceof $extractorClass3);
+               $this->assertTrue($extractorInstances[2] instanceof $extractorClass1);
+       }
+
+       /**
+        * @test
+        */
+       public function registeredExtractorClassWithSamePriorityAreReturnedInSameOrderAsTheyWereAdded() {
+
+               $extractorClass1 = 'b70551b2b2db62b6b15a9bbfcbd50614';
+               $extractorObject1 = $this->getMock(\TYPO3\CMS\Core\Resource\Index\ExtractorInterface::class, array(), array(), $extractorClass1);
+               $extractorObject1->expects($this->any())->method('getPriority')->will($this->returnValue(1));
+
+               $extractorClass2 = 'ac318f1659d278b79b38262f23a78d5d';
+               $extractorObject2 = $this->getMock(\TYPO3\CMS\Core\Resource\Index\ExtractorInterface::class, array(), array(), $extractorClass2);
+               $extractorObject2->expects($this->any())->method('getPriority')->will($this->returnValue(1));
+
+               $createdExtractorInstances = array(
+                       array($extractorClass1, $extractorObject1),
+                       array($extractorClass2, $extractorObject2),
+               );
+
+               $extractorRegistry = $this->getMockExtractorRegistry($createdExtractorInstances);
+               $extractorRegistry->registerExtractionService($extractorClass1);
+               $extractorRegistry->registerExtractionService($extractorClass2);
+
+               $extractorInstances = $extractorRegistry->getExtractors();
+               $this->assertTrue($extractorInstances[0] instanceof $extractorClass1);
+               $this->assertTrue($extractorInstances[1] instanceof $extractorClass2);
+       }
+
+       /**
+        * Initialize an ExtractorRegistry and mock createExtractorInstance()
+        *
+        * @param array $createsExtractorInstances
+        * @return \PHPUnit_Framework_MockObject_MockObject|\TYPO3\CMS\Core\Resource\Index\ExtractorRegistry
+        */
+       protected function getMockExtractorRegistry(array $createsExtractorInstances = array()) {
+               $extractorRegistry = $this->getMockBuilder(\TYPO3\CMS\Core\Resource\Index\ExtractorRegistry::class)
+                       ->setMethods(array('createExtractorInstance'))
+                       ->getMock();
+
+               if (count($createsExtractorInstances) > 0) {
+                       $extractorRegistry->expects($this->any())
+                               ->method('createExtractorInstance')
+                               ->will($this->returnValueMap($createsExtractorInstances));
+               }
+
+               return $extractorRegistry;
+       }
+
+}
index 5fbe75e..a19fdef 100644 (file)
@@ -20,7 +20,7 @@ namespace TYPO3\CMS\Core\Tests\Unit\Resource\Rendering;
 class RendererRegistryTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
 
        /**
-        * Initialize an RendererRegistry and mock createRendererInstance()
+        * Initialize a RendererRegistry and mock createRendererInstance()
         *
         * @param array $createsRendererInstances
         * @return \PHPUnit_Framework_MockObject_MockObject|\TYPO3\CMS\Core\Resource\Rendering\RendererRegistry