[BUGFIX] Replace the table definition manipulation by signals 42/24942/8
authorThomas Maroschik <tmaroschik@dfau.de>
Mon, 21 Oct 2013 17:47:20 +0000 (19:47 +0200)
committerThomas Maroschik <tmaroschik@dfau.de>
Thu, 19 Dec 2013 06:51:30 +0000 (07:51 +0100)
During installation of extensions the Extension Manager does
not take the Category API into account. The code to do so is
present in the Install Tool in the Database Compare Tool. It is
cumbersome to switch to the install tool to update the database
in order to use the category fields. The install tool and extension
manager currently need to know which components manipulate
the table definitions and this is bad coupling of components
which shouldn't know each other.

This fix replaces the individual calls to the components by two
signals and thus a generic approach.

Fixes: #53016
Releases: 6.2
Change-Id: Ibaea293b96fb1b8df1eacdcdd2f98acf74fb155b
Reviewed-on: https://review.typo3.org/24942
Reviewed-by: Stefan Neufeind
Tested-by: Stefan Neufeind
Reviewed-by: Fabien Udriot
Tested-by: Fabien Udriot
Reviewed-by: Thomas Maroschik
Tested-by: Thomas Maroschik
typo3/sysext/core/Classes/Cache/Cache.php
typo3/sysext/core/Classes/Category/CategoryRegistry.php
typo3/sysext/core/Tests/Unit/Category/CategoryRegistryTest.php
typo3/sysext/extensionmanager/Classes/Utility/InstallUtility.php
typo3/sysext/extensionmanager/Tests/Unit/Utility/InstallUtilityTest.php
typo3/sysext/extensionmanager/ext_localconf.php
typo3/sysext/install/Classes/Controller/Action/Tool/UpdateWizard.php
typo3/sysext/install/Classes/Service/CachingFrameworkDatabaseSchemaService.php [new file with mode: 0644]
typo3/sysext/install/Classes/Service/Exception/UnexpectedSignalReturnValueTypeException.php [new file with mode: 0644]
typo3/sysext/install/Classes/Service/SqlExpectedSchemaService.php
typo3/sysext/install/ext_localconf.php

index c8f314e..8198f21 100644 (file)
@@ -98,4 +98,18 @@ class Cache {
                return $tableDefinitions;
        }
 
+       /**
+        * A slot method to inject the required caching framework database tables to the
+        * tables defintions string
+        *
+        * @param array $sqlString
+        * @param string $extensionKey
+        * @return array
+        */
+       public function addCachingFrameworkRequiredDatabaseSchemaToTablesDefintion(array $sqlString, $extensionKey) {
+               $GLOBALS['typo3CacheManager']->setCacheConfigurations($GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']);
+               $sqlString[] = static::getDatabaseTableDefinitions();
+               return array('sqlString' => $sqlString, 'extensionKey' => $extensionKey);
+       }
+
 }
index 1cd1407..a77e25c 100644 (file)
@@ -88,8 +88,8 @@ class CategoryRegistry implements \TYPO3\CMS\Core\SingletonInterface {
                        throw new \InvalidArgumentException('TYPO3\\CMS\\Core\\Category\\CategoryRegistry No tableName given.', 1369122038);
                }
 
-               // Makes sure there is an existing table configuration and nothing registered yet:
-               if (isset($GLOBALS['TCA'][$tableName]) && !$this->isRegistered($tableName, $fieldName)) {
+               // Makes sure nothing was registered yet.
+               if (!$this->isRegistered($tableName, $fieldName)) {
                        $this->registry[$extensionKey][$tableName][$fieldName] = $options;
                        $result = TRUE;
                }
@@ -372,4 +372,29 @@ class CategoryRegistry implements \TYPO3\CMS\Core\SingletonInterface {
                        \TYPO3\CMS\Core\Utility\ExtensionManagementUtility::addTCAcolumns($tableName, $columns);
                }
        }
+
+       /**
+        * A slot method to inject the required category database fields to the
+        * tables defintion string
+        *
+        * @param array $sqlString
+        * @return array
+        */
+       public function addCategoryDatabaseSchemaToTablesDefintion(array $sqlString) {
+               $sqlString[] = $this->getDatabaseTableDefinitions();
+               return array('sqlString' => $sqlString);
+       }
+
+       /**
+        * A slot method to inject the required category database fields of an
+        * extension to the tables defintion string
+        *
+        * @param array $sqlString
+        * @param string $extensionKey
+        * @return array
+        */
+       public function addExtensionCategoryDatabaseSchemaToTablesDefintion(array $sqlString, $extensionKey) {
+               $sqlString[] = $this->getDatabaseTableDefinition($extensionKey);
+               return array('sqlString' => $sqlString, 'extensionKey' => $extensionKey);
+       }
 }
index e4017d7..371af39 100644 (file)
@@ -79,13 +79,6 @@ class CategoryRegistryTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
 
        /**
         * @test
-        */
-       public function doesAddReturnFalseOnUndefinedTable() {
-               $this->assertFalse($this->fixture->add('test_extension_a', 'undefined_table', 'categories'));
-       }
-
-       /**
-        * @test
         * @expectedException \InvalidArgumentException
         * @expectedExceptionCode 1369122038
         */
index bbe5a7a..432ddad 100644 (file)
@@ -123,7 +123,7 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface {
                        $this->loadExtension($extensionKey);
                }
                $this->reloadCaches();
-               $this->processCachingFrameworkUpdates();
+               $this->processRuntimeDatabaseUpdates($extensionKey);
                $this->saveDefaultConfiguration($extension['key']);
        }
 
@@ -240,20 +240,40 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface {
        }
 
        /**
-        * Gets all registered caches and creates required caching framework tables.
+        * Gets all database updates due to runtime configuration, like caching framework or
+        * category api for example
         *
-        * @return void
+        * @param string $extensionKey
         */
-       protected function processCachingFrameworkUpdates() {
-               $extTablesSqlContent = '';
-
-               // @TODO: This should probably moved to TYPO3\CMS\Core\Cache\Cache->getDatabaseTableDefinitions ?!
-               $GLOBALS['typo3CacheManager']->setCacheConfigurations($GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']);
-               $extTablesSqlContent .= \TYPO3\CMS\Core\Cache\Cache::getDatabaseTableDefinitions();
+       protected function processRuntimeDatabaseUpdates($extensionKey) {
+               $sqlString = $this->emitTablesDefinitionIsBeingBuiltSignal($extensionKey);
+               if (!empty($sqlString)) {
+                       $this->updateDbWithExtTablesSql(implode(LF . LF . LF . LF, $sqlString));
+               }
+       }
 
-               if ($extTablesSqlContent !== '') {
-                       $this->updateDbWithExtTablesSql($extTablesSqlContent);
+       /**
+        * Emits a signal to manipulate the tables definitions
+        *
+        * @param string $extensionKey
+        * @return mixed
+        * @throws \TYPO3\CMS\Extensionmanager\Exception\ExtensionManagerException
+        */
+       protected function emitTablesDefinitionIsBeingBuiltSignal($extensionKey) {
+               $signalReturn = $this->signalSlotDispatcher->dispatch(__CLASS__, 'tablesDefinitionIsBeingBuilt', array('sqlString' => array(), 'extensionKey' => $extensionKey));
+               $sqlString = $signalReturn['sqlString'];
+               if (!is_array($sqlString)) {
+                       throw new \TYPO3\CMS\Extensionmanager\Exception\ExtensionManagerException(
+                               sprintf(
+                                       'The signal %s of class %s returned a value of type %s, but array was expected.',
+                                       'tablesDefinitionIsBeingBuilt',
+                                       __CLASS__,
+                                       gettype($sqlString)
+                               ),
+                               1382360258
+                       );
                }
+               return $sqlString;
        }
 
        /**
@@ -263,7 +283,7 @@ class InstallUtility implements \TYPO3\CMS\Core\SingletonInterface {
         */
        public function reloadCaches() {
                \TYPO3\CMS\Core\Utility\ExtensionManagementUtility::removeCacheFiles();
-               \TYPO3\CMS\Core\Core\Bootstrap::getInstance()->reloadTypo3LoadedExtAndClassLoaderAndExtLocalconf();
+               \TYPO3\CMS\Core\Core\Bootstrap::getInstance()->reloadTypo3LoadedExtAndClassLoaderAndExtLocalconf()->loadExtensionTables();
        }
 
        /**
index d362123..3724bf2 100644 (file)
@@ -64,6 +64,7 @@ class InstallUtilityTest extends \TYPO3\CMS\Extbase\Tests\Unit\BaseTestCase {
                                'loadExtension',
                                'unloadExtension',
                                'processDatabaseUpdates',
+                               'processRuntimeDatabaseUpdates',
                                'reloadCaches',
                                'processCachingFrameworkUpdates',
                                'saveDefaultConfiguration',
@@ -119,20 +120,10 @@ class InstallUtilityTest extends \TYPO3\CMS\Extbase\Tests\Unit\BaseTestCase {
        /**
         * @test
         */
-       public function installCallsProcessDatabaseUpdates() {
+       public function installCallsProcessRuntimeDatabaseUpdates() {
                $this->installMock->expects($this->once())
-                       ->method('processDatabaseUpdates')
-                       ->with($this->extensionData);
-
-               $this->installMock->install($this->extensionKey);
-       }
-
-       /**
-        * @test
-        */
-       public function installCallsProcessCachingFrameworkUpdates() {
-               $this->installMock->expects($this->once())
-                       ->method('processCachingFrameworkUpdates');
+                       ->method('processRuntimeDatabaseUpdates')
+                       ->with($this->extensionKey);
 
                $this->installMock->install($this->extensionKey);
        }
index f339e04..ff21ce1 100644 (file)
@@ -21,5 +21,17 @@ if (TYPO3_MODE === 'BE') {
                        'TYPO3\\CMS\\Core\\Package\\PackageManager',
                        'scanAvailablePackages'
                );
+               $signalSlotDispatcher->connect(
+                       'TYPO3\\CMS\\Extensionmanager\\Utility\\InstallUtility',
+                       'tablesDefinitionIsBeingBuilt',
+                       'TYPO3\\CMS\\Core\\Cache\\Cache',
+                       'addCachingFrameworkRequiredDatabaseSchemaToTablesDefintion'
+               );
+               $signalSlotDispatcher->connect(
+                       'TYPO3\\CMS\\Extensionmanager\\Utility\\InstallUtility',
+                       'tablesDefinitionIsBeingBuilt',
+                       'TYPO3\\CMS\\Core\\Category\\CategoryRegistry',
+                       'addExtensionCategoryDatabaseSchemaToTablesDefintion'
+               );
        }
 }
index c5287a2..94a9743 100644 (file)
@@ -258,9 +258,9 @@ class UpdateWizard extends Action\AbstractAction implements Action\ActionInterfa
                /** @var $sqlHandler \TYPO3\CMS\Install\Service\SqlSchemaMigrationService */
                $sqlHandler = $this->objectManager->get('TYPO3\\CMS\\Install\\Service\\SqlSchemaMigrationService');
 
-               /** @var \TYPO3\CMS\Install\Service\SqlExpectedSchemaService $expectedSchemaService */
-               $expectedSchemaService = $this->objectManager->get('TYPO3\\CMS\\Install\\Service\\SqlExpectedSchemaService');
-               $expectedSchemaString = $expectedSchemaService->getCachingFrameworkRequiredDatabaseSchema();
+               /** @var \TYPO3\CMS\Install\Service\CachingFrameworkDatabaseSchemaService $cachingFrameworkDatabaseSchemaService */
+               $cachingFrameworkDatabaseSchemaService = $this->objectManager->get('TYPO3\\CMS\\Install\\Service\\CachingFrameworkDatabaseSchemaService');
+               $expectedSchemaString = $cachingFrameworkDatabaseSchemaService->getCachingFrameworkRequiredDatabaseSchema();
                $cleanedExpectedSchemaString = implode(LF, $sqlHandler->getStatementArray($expectedSchemaString, TRUE, '^CREATE TABLE '));
                $neededTableDefinition = $sqlHandler->getFieldDefinitions_fileContent($cleanedExpectedSchemaString);
                $currentTableDefinition = $sqlHandler->getFieldDefinitions_database();
diff --git a/typo3/sysext/install/Classes/Service/CachingFrameworkDatabaseSchemaService.php b/typo3/sysext/install/Classes/Service/CachingFrameworkDatabaseSchemaService.php
new file mode 100644 (file)
index 0000000..84d5e32
--- /dev/null
@@ -0,0 +1,84 @@
+<?php
+namespace TYPO3\CMS\Install\Service;
+
+/***************************************************************
+ *  Copyright notice
+ *
+ *  (c) 2013 Thomas Maroschik
+ *  All rights reserved
+ *
+ *  This script is part of the TYPO3 project. The TYPO3 project is
+ *  free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  The GNU General Public License can be found at
+ *  http://www.gnu.org/copyleft/gpl.html.
+ *  A copy is found in the textfile GPL.txt and important notices to the license
+ *  from the author is found in LICENSE.txt distributed with these scripts.
+ *
+ *
+ *  This script is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  This copyright notice MUST APPEAR in all copies of the script!
+ ***************************************************************/
+
+/**
+ * This service provides the sql schema for the caching framework
+ */
+class CachingFrameworkDatabaseSchemaService {
+
+       /**
+        * Get schema SQL of required cache framework tables.
+        *
+        * This method needs ext_localconf and ext_tables loaded!
+        *
+        * This is a hack, but there was no smarter solution with current cache configuration setup:
+        * ToolController sets the extbase caches to NullBackend to ensure the install tool does not
+        * cache anything. The CacheManager gets the required SQL from database backends only, so we need to
+        * temporarily 'fake' the standard db backends for extbase caches so they are respected.
+        *
+        * Additionally, the extbase_object cache is already in use and instantiated, and the CacheManager singleton
+        * does not allow overriding this definition. The only option at the moment is to 'fake' another cache with
+        * a different name, and then substitute this name in the sql content with the real one.
+        *
+        * @TODO: http://forge.typo3.org/issues/54498
+        * @TODO: It might be possible to reduce this ugly construct by circumventing the 'singleton' of CacheManager by using 'new'
+        *
+        * @return string Cache framework SQL
+        */
+       public function getCachingFrameworkRequiredDatabaseSchema() {
+               $cacheConfigurationBackup = $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations'];
+               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']['extbase_datamapfactory_datamap'] = array();
+               $extbaseObjectFakeName = uniqid('extbase_object');
+               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations'][$extbaseObjectFakeName] = array();
+               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']['extbase_reflection'] = array();
+               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']['extbase_typo3dbbackend_tablecolumns'] = array();
+               /** @var \TYPO3\CMS\Core\Cache\CacheManager $cacheManager */
+               $cacheManager = $GLOBALS['typo3CacheManager'];
+               $cacheManager->setCacheConfigurations($GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']);
+               $cacheSqlString = \TYPO3\CMS\Core\Cache\Cache::getDatabaseTableDefinitions();
+               $sqlString = str_replace($extbaseObjectFakeName, 'extbase_object', $cacheSqlString);
+               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations'] = $cacheConfigurationBackup;
+               $cacheManager->setCacheConfigurations($GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']);
+
+               return $sqlString;
+       }
+
+       /**
+        * A slot method to inject the required caching framework database tables to the
+        * tables defintions string
+        *
+        * @param array $sqlString
+        * @return array
+        */
+       public function addCachingFrameworkRequiredDatabaseSchemaToTablesDefintion(array $sqlString) {
+               $sqlString[] = $this->getCachingFrameworkRequiredDatabaseSchema();
+               return array('sqlString' => $sqlString);
+       }
+
+}
diff --git a/typo3/sysext/install/Classes/Service/Exception/UnexpectedSignalReturnValueTypeException.php b/typo3/sysext/install/Classes/Service/Exception/UnexpectedSignalReturnValueTypeException.php
new file mode 100644 (file)
index 0000000..b6ea001
--- /dev/null
@@ -0,0 +1,32 @@
+<?php
+namespace TYPO3\CMS\Install\Service\Exception;
+
+/***************************************************************
+ *  Copyright notice
+ *
+ *  (c) 2013 Christian Kuhn <lolli@schwarzbu.ch>
+ *  All rights reserved
+ *
+ *  This script is part of the TYPO3 project. The TYPO3 project is
+ *  free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  The GNU General Public License can be found at
+ *  http://www.gnu.org/copyleft/gpl.html.
+ *
+ *  This script is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  This copyright notice MUST APPEAR in all copies of the script!
+ ***************************************************************/
+
+/**
+ * An exception thrown if the return value type of a signal is not the expected one.
+ */
+class UnexpectedSignalReturnValueTypeException extends CoreVersionServiceException {
+
+}
\ No newline at end of file
index 253e83f..5cdee62 100644 (file)
@@ -43,6 +43,12 @@ class SqlExpectedSchemaService {
        protected $objectManager = NULL;
 
        /**
+        * @var \TYPO3\CMS\Extbase\SignalSlot\Dispatcher
+        * @inject
+        */
+       protected $signalSlotDispatcher;
+
+       /**
         * Get expected schema array
         *
         * @return array Expected schema
@@ -81,50 +87,31 @@ class SqlExpectedSchemaService {
                        }
                }
 
-               // Add caching framework sql definition
-               $sqlString[] = $this->getCachingFrameworkRequiredDatabaseSchema();
-
-               // Add category registry sql definition
-               $sqlString[] = \TYPO3\CMS\Core\Category\CategoryRegistry::getInstance()->getDatabaseTableDefinitions();
+               $sqlString = $this->emitTablesDefinitionIsBeingBuiltSignal($sqlString);
 
                return implode(LF . LF . LF . LF, $sqlString);
        }
 
        /**
-        * Get schema SQL of required cache framework tables.
-        *
-        * This method needs ext_localconf and ext_tables loaded!
+        * Emits a signal to manipulate the tables definitions
         *
-        * This is a hack, but there was no smarter solution with current cache configuration setup:
-        * ToolController sets the extbase caches to NullBackend to ensure the install tool does not
-        * cache anything. The CacheManager gets the required SQL from database backends only, so we need to
-        * temporarily 'fake' the standard db backends for extbase caches so they are respected.
-        *
-        * Additionally, the extbase_object cache is already in use and instantiated, and the CacheManager singleton
-        * does not allow overriding this definition. The only option at the moment is to 'fake' another cache with
-        * a different name, and then substitute this name in the sql content with the real one.
-        *
-        * @TODO: This construct needs to be improved. It does not recognise if some custom ext overwrote the extbase cache config
-        * @TODO: Solve this as soon as cache configuration is separated from ext_localconf / ext_tables
-        * @TODO: It might be possible to reduce this ugly construct by circumventing the 'singleton' of CacheManager by using 'new'
-        *
-        * @return string Cache framework SQL
+        * @param array $sqlString
+        * @return mixed
         */
-       public function getCachingFrameworkRequiredDatabaseSchema() {
-               $cacheConfigurationBackup = $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations'];
-               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']['extbase_datamapfactory_datamap'] = array();
-               $extbaseObjectFakeName = uniqid('extbase_object');
-               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations'][$extbaseObjectFakeName] = array();
-               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']['extbase_reflection'] = array();
-               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']['extbase_typo3dbbackend_tablecolumns'] = array();
-               /** @var \TYPO3\CMS\Core\Cache\CacheManager $cacheManager */
-               $cacheManager = $GLOBALS['typo3CacheManager'];
-               $cacheManager->setCacheConfigurations($GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']);
-               $cacheSqlString = \TYPO3\CMS\Core\Cache\Cache::getDatabaseTableDefinitions();
-               $sqlString = str_replace($extbaseObjectFakeName, 'extbase_object', $cacheSqlString);
-               $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations'] = $cacheConfigurationBackup;
-               $cacheManager->setCacheConfigurations($GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']);
-
+       protected function emitTablesDefinitionIsBeingBuiltSignal(array $sqlString) {
+               $signalReturn = $this->signalSlotDispatcher->dispatch(__CLASS__, 'tablesDefinitionIsBeingBuilt', array('sqlString' => $sqlString));
+               $sqlString = $signalReturn['sqlString'];
+               if (!is_array($sqlString)) {
+                       throw new Exception\UnexpectedSignalReturnValueTypeException(
+                               sprintf(
+                                       'The signal %s of class %s returned a value of type %s, but array was expected.',
+                                       'tablesDefinitionIsBeingBuilt',
+                                       __CLASS__,
+                                       gettype($sqlString)
+                               ),
+                               1382351456
+                       );
+               }
                return $sqlString;
        }
 }
index 06007b4..52a8f5f 100644 (file)
@@ -52,3 +52,17 @@ $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['ext/install']['update']['sysext_file_
 
 // Version 4.7: Migrate the flexforms of MediaElement
 $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['ext/install']['update']['mediaElementFlexform'] = 'TYPO3\\CMS\\Install\\Updates\\MediaFlexformUpdate';
+
+$signalSlotDispatcher = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance('TYPO3\\CMS\\Extbase\\SignalSlot\\Dispatcher');
+$signalSlotDispatcher->connect(
+       'TYPO3\\CMS\\Install\\Service\\SqlExpectedSchemaService',
+       'tablesDefinitionIsBeingBuilt',
+       'TYPO3\\CMS\\Install\\Service\\CachingFrameworkDatabaseSchemaService',
+       'addCachingFrameworkRequiredDatabaseSchemaToTablesDefintion'
+);
+$signalSlotDispatcher->connect(
+       'TYPO3\\CMS\\Install\\Service\\SqlExpectedSchemaService',
+       'tablesDefinitionIsBeingBuilt',
+       'TYPO3\\CMS\\Core\\Category\\CategoryRegistry',
+       'addCategoryDatabaseSchemaToTablesDefintion'
+);