[!!!][FOLLOWUP][TASK] Simplify PackageManagement 26/39626/4
authorBenjamin Mack <benni@typo3.org>
Wed, 20 May 2015 05:45:45 +0000 (13:45 +0800)
committerAndreas Fernandez <typo3@scripting-base.de>
Fri, 5 Jun 2015 14:28:49 +0000 (16:28 +0200)
This change cleans up more unneeded functionality, and adds / fixes
some comments and streamlines some function calls within the Package
management itself.

Resolves: #67027
Releases: master
Change-Id: I26beddeeb8f014089545aa153fa38494e5bbd3d4
Reviewed-on: http://review.typo3.org/39626
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Andreas Fernandez <typo3@scripting-base.de>
Tested-by: Andreas Fernandez <typo3@scripting-base.de>
typo3/sysext/core/Classes/Package/Exception/InvalidPackageManifestException.php
typo3/sysext/core/Classes/Package/Package.php
typo3/sysext/core/Classes/Package/PackageManager.php
typo3/sysext/core/Tests/Unit/Package/PackageTest.php

index bd492fa..0d3f5e0 100644 (file)
@@ -15,7 +15,7 @@ namespace TYPO3\CMS\Core\Package\Exception;
  */
 
 /**
- * An "Invalid Package Key" exception
+ * An "Invalid Package Manifest" exception
  * Adapted from FLOW for TYPO3 CMS
  */
 class InvalidPackageManifestException extends \TYPO3\CMS\Core\Package\Exception {
index 9c34f3d..45da4ab 100644 (file)
@@ -14,8 +14,6 @@ namespace TYPO3\CMS\Core\Package;
  * The TYPO3 project - inspiring people to share!
  */
 
-use TYPO3\CMS\Core\Utility\PathUtility;
-
 /**
  * A Package representing the details of an extension and/or a composer package
  * Adapted from FLOW for TYPO3 CMS
@@ -33,11 +31,6 @@ class Package implements PackageInterface {
        protected $classAliases;
 
        /**
-        * @var array
-        */
-       protected $ignoredClassNames = array();
-
-       /**
         * If this package is part of factory default, it will be activated
         * during first installation.
         *
@@ -60,26 +53,14 @@ class Package implements PackageInterface {
        protected $packageKey;
 
        /**
-        * @var string
-        */
-       protected $manifestPath = '';
-
-       /**
         * Full path to this package's main directory
         * @var string
         */
        protected $packagePath;
 
        /**
-        * Full path to this package's PSR-0 class loader entry point
-        * @var string
-        */
-       protected $classesPath;
-
-       /**
         * If this package is protected and therefore cannot be deactivated or deleted
         * @var bool
-        * @api
         */
        protected $protected = FALSE;
 
@@ -127,10 +108,9 @@ class Package implements PackageInterface {
                }
                $this->packageManager = $packageManager;
                $this->packageKey = $packageKey;
-               $this->packagePath = PathUtility::sanitizeTrailingSeparator($packagePath);
-               $this->classesPath = PathUtility::sanitizeTrailingSeparator($this->packagePath . self::DIRECTORY_CLASSES);
+               $this->packagePath = $packagePath;
                try {
-                       $this->getComposerManifest();
+                       $this->composerManifest = $this->packageManager->getComposerManifest($this->packagePath);
                } catch (Exception\MissingPackageManifestException $exception) {
                        if (!$this->loadExtensionEmconf()) {
                                throw new Exception\InvalidPackageManifestException('No valid ext_emconf.php file found for package "' . $packageKey . '".', 1360403545);
@@ -146,7 +126,7 @@ class Package implements PackageInterface {
         * @return void
         */
        protected function loadFlagsFromComposerManifest() {
-               $extraFlags = $this->getComposerManifest('extra');
+               $extraFlags = $this->getValueFromComposerManifest('extra');
                if ($extraFlags !== NULL && isset($extraFlags->{"typo3/cms"}->{"Package"})) {
                        foreach ($extraFlags->{"typo3/cms"}->{"Package"} as $flagName => $flagValue) {
                                if (property_exists($this, $flagName)) {
@@ -221,22 +201,13 @@ class Package implements PackageInterface {
        }
 
        /**
-        * Returns the full path to the packages Composer manifest
-        *
-        * @return string
-        */
-       public function getManifestPath() {
-               return $this->packagePath . $this->manifestPath;
-       }
-
-       /**
         * Returns the full path to this package's Classes directory
         *
         * @return string Path to this package's Classes directory
         * @api
         */
        public function getClassesPath() {
-               return $this->classesPath;
+               return $this->packagePath . self::DIRECTORY_CLASSES;
        }
 
        /**
@@ -260,6 +231,7 @@ class Package implements PackageInterface {
        }
 
        /**
+        * Fetches MetaData information from ext_emconf.php, used for resolving dependencies as well
         * @return bool
         */
        protected function loadExtensionEmconf() {
@@ -278,7 +250,9 @@ class Package implements PackageInterface {
        }
 
        /**
+        * Fetches information from ext_emconf.php and maps it so it is treated as it would come from composer.json
         *
+        * @return void
         */
        protected function mapExtensionManagerConfigurationToComposerManifest() {
                if (is_array($this->extensionManagerConfiguration)) {
@@ -329,9 +303,9 @@ class Package implements PackageInterface {
        public function getPackageMetaData() {
                if ($this->packageMetaData === NULL) {
                        $this->packageMetaData = new MetaData($this->getPackageKey());
-                       $this->packageMetaData->setDescription($this->getComposerManifest('description'));
-                       $this->packageMetaData->setVersion($this->getComposerManifest('version'));
-                       $requirements = $this->getComposerManifest('require');
+                       $this->packageMetaData->setDescription($this->getValueFromComposerManifest('description'));
+                       $this->packageMetaData->setVersion($this->getValueFromComposerManifest('version'));
+                       $requirements = $this->getValueFromComposerManifest('require');
                        if ($requirements !== NULL) {
                                foreach ($requirements as $requirement => $version) {
                                        if ($this->packageRequirementIsComposerPackage($requirement) === FALSE) {
@@ -343,7 +317,7 @@ class Package implements PackageInterface {
                                        $this->packageMetaData->addConstraint($constraint);
                                }
                        }
-                       $suggestions = $this->getComposerManifest('suggest');
+                       $suggestions = $this->getValueFromComposerManifest('suggest');
                        if ($suggestions !== NULL) {
                                foreach ($suggestions as $suggestion => $version) {
                                        if ($this->packageRequirementIsComposerPackage($suggestion) === FALSE) {
@@ -360,21 +334,23 @@ class Package implements PackageInterface {
        }
 
        /**
+        * Returns an array of packages this package replaces
+        *
         * @return array
         */
        public function getPackageReplacementKeys() {
                // The cast to array is required since the manifest returns data with type mixed
-               return (array)$this->getComposerManifest('replace') ?: array();
+               return (array)$this->getValueFromComposerManifest('replace') ?: array();
        }
 
        /**
-        * Returns the PHP namespace of classes in this package.
+        * Returns the PHP namespace of classes in this package, also uses a fallback for extensions without having a "."
+        * in the package key.
         *
         * @return string
-        * @throws Exception\InvalidPackageStateException
         */
        public function getNamespace() {
-               if(!$this->namespace) {
+               if (!$this->namespace) {
                        $packageKey = $this->getPackageKey();
                        if (strpos($packageKey, '.') === FALSE) {
                                // Old school with unknown vendor name
@@ -387,25 +363,6 @@ class Package implements PackageInterface {
        }
 
        /**
-        * @param array $classFiles
-        * @return array
-        */
-       protected function filterClassFiles(array $classFiles) {
-               $classesNotMatchingClassRule = array_filter(array_keys($classFiles), function($className) {
-                       return preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\\\x7f-\xff]*$/', $className) !== 1;
-               });
-               foreach ($classesNotMatchingClassRule as $forbiddenClassName) {
-                       unset($classFiles[$forbiddenClassName]);
-               }
-               foreach ($this->ignoredClassNames as $ignoredClassName) {
-                       if (isset($classFiles[$ignoredClassName])) {
-                               unset($classFiles[$ignoredClassName]);
-                       }
-               }
-               return $classFiles;
-       }
-
-       /**
         * @return array
         */
        public function getClassFilesFromAutoloadRegistry() {
@@ -417,12 +374,14 @@ class Package implements PackageInterface {
        }
 
        /**
+        * Fetches class aliases registered via Migrations/Code/ClassAliasMap.php
         *
+        * @return array
         */
        public function getClassAliases() {
                if (!is_array($this->classAliases)) {
                        try {
-                               $extensionClassAliasMapPathAndFilename = $this->getPackagePath() . 'Migrations/Code/ClassAliasMap.php';
+                               $extensionClassAliasMapPathAndFilename = $this->packagePath . 'Migrations/Code/ClassAliasMap.php';
                                if (@file_exists($extensionClassAliasMapPathAndFilename)) {
                                        $this->classAliases = require $extensionClassAliasMapPathAndFilename;
                                }
@@ -450,65 +409,23 @@ class Package implements PackageInterface {
        }
 
        /**
-        * Returns contents of Composer manifest - or part there of.
+        * Returns contents of Composer manifest - or part there of if a key is given.
         *
         * @param string $key Optional. Only return the part of the manifest indexed by 'key'
         * @return mixed|NULL
         * @see json_decode for return values
         */
-       public function getComposerManifest($key = NULL) {
-               if (!isset($this->composerManifest)) {
-                       $this->composerManifest = PackageManager::getComposerManifest($this->getManifestPath());
+       public function getValueFromComposerManifest($key = NULL) {
+               if ($key === NULL) {
+                       return $this->composerManifest;
                }
 
-               return PackageManager::getComposerManifest($this->getManifestPath(), $key, $this->composerManifest);
-       }
-
-       /**
-        * Builds and returns an array of class names => file names of all
-        * *.php files in the package's Classes directory and its sub-
-        * directories.
-        *
-        * @param string $classesPath Base path acting as the parent directory for potential class files
-        * @param string $extraNamespaceSegment A PHP class namespace segment which should be inserted like so: \TYPO3\PackageKey\{namespacePrefix\}PathSegment\PathSegment\Filename
-        * @param string $subDirectory Used internally
-        * @param int $recursionLevel Used internally
-        * @return array
-        * @throws Exception if recursion into directories was too deep or another error occurred
-        */
-       protected function buildArrayOfClassFiles($classesPath, $extraNamespaceSegment = '', $subDirectory = '', $recursionLevel = 0) {
-               $classFiles = array();
-               $currentPath = $classesPath . $subDirectory;
-               $currentRelativePath = substr($currentPath, strlen($this->packagePath));
-
-               if (!is_dir($currentPath)) {
-                       return array();
-               }
-               if ($recursionLevel > 100) {
-                       throw new Exception('Recursion too deep.', 1166635495);
-               }
-
-               try {
-                       $classesDirectoryIterator = new \DirectoryIterator($currentPath);
-                       while ($classesDirectoryIterator->valid()) {
-                               $filename = $classesDirectoryIterator->getFilename();
-                               if ($filename[0] !== '.') {
-                                       if (is_dir($currentPath . $filename)) {
-                                               $classFiles = array_merge($classFiles, $this->buildArrayOfClassFiles($classesPath, $extraNamespaceSegment, $subDirectory . $filename . '/', ($recursionLevel + 1)));
-                                       } else {
-                                               if (substr($filename, -4, 4) === '.php') {
-                                                       $className = (str_replace('/', '\\', ($extraNamespaceSegment . substr($currentPath, strlen($classesPath)) . substr($filename, 0, -4))));
-                                                       $classFiles[$className] = $currentRelativePath . $filename;
-                                               }
-                                       }
-                               }
-                               $classesDirectoryIterator->next();
-                       }
-
-               } catch (\Exception $exception) {
-                       throw new Exception($exception->getMessage(), 1166633721);
+               if (isset($this->composerManifest->{$key})) {
+                       $value = $this->composerManifest->{$key};
+               } else {
+                       $value = NULL;
                }
-               return $classFiles;
+               return $value;
        }
 
        /**
index 420a6fe..4ded5a2 100644 (file)
@@ -14,6 +14,7 @@ namespace TYPO3\CMS\Core\Package;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Compatibility\LoadedExtensionArrayElement;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\PathUtility;
 
@@ -211,7 +212,7 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
        }
 
        /**
-        *
+        * Saves the current state of all relevant information to the TYPO3 Core Cache
         */
        protected function saveToPackageCache() {
                $cacheEntryIdentifier = $this->getCacheEntryIdentifier();
@@ -355,7 +356,7 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
 
                $packagePaths = $this->scanLegacyExtensions();
                foreach ($this->packagesBasePaths as $packagesBasePath) {
-                       $this->scanPackagesInPath($packagesBasePath, $packagePaths);
+                       $packagePaths = $this->scanPackagesInPath($packagesBasePath, $packagePaths);
                }
 
                foreach ($packagePaths as $composerManifestPath) {
@@ -368,7 +369,7 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
                                }
                        }
                        try {
-                               $composerManifest = self::getComposerManifest($composerManifestPath);
+                               $composerManifest = $this->getComposerManifest($composerManifestPath);
                                $packageKey = $this->getPackageKeyFromManifest($composerManifest, $packagePath, $packagesBasePath);
                                $this->composerNameToPackageKeyMap[strtolower($composerManifest->name)] = $packageKey;
                                $this->packageStatesConfiguration['packages'][$packageKey]['composerName'] = $composerManifest->name;
@@ -396,6 +397,8 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
        }
 
        /**
+        * Fetches all directories from sysext/global/local locations and checks if the extension contains a ext_emconf.php
+        *
         * @param array $collectedExtensionPaths
         * @return array
         */
@@ -435,11 +438,11 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
         * @return bool TRUE if a composer.json exists or FALSE if none exists
         */
        protected function hasComposerManifestFile($packagePath) {
-               // If an ext_emconf.php file is found, we don't need to look deeper
-               if (file_exists($packagePath . '/ext_emconf.php')) {
+               // If an ext_emconf.php file is found, we don't need to look further
+               if (file_exists($packagePath . 'ext_emconf.php')) {
                        return FALSE;
                }
-               if (file_exists($packagePath . '/composer.json')) {
+               if (file_exists($packagePath . 'composer.json')) {
                        return TRUE;
                }
                return FALSE;
@@ -496,7 +499,8 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
         * @param PackageInterface $package The Package to be registered
         * @param bool $sortAndSave allows for not saving packagestates when used in loops etc.
         * @return PackageInterface
-        * @throws Exception\CorruptPackageException
+        * @throws Exception\InvalidPackageStateException
+        * @throws Exception\PackageStatesFileNotWritableException
         */
        public function registerPackage(PackageInterface $package, $sortAndSave = TRUE) {
                $packageKey = $package->getPackageKey();
@@ -629,7 +633,12 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
        }
 
        /**
+        * Deactivates a package and updates the packagestates configuration
+        *
         * @param string $packageKey
+        * @throws Exception\PackageStatesFileNotWritableException
+        * @throws Exception\ProtectedPackageKeyException
+        * @throws Exception\UnknownPackageException
         */
        public function deactivatePackage($packageKey) {
                $this->sortAvailablePackagesByDependencies();
@@ -687,13 +696,13 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
                $this->runtimeActivatedPackages[$package->getPackageKey()] = $package;
                $this->classLoader->addActivePackage($package);
                if (!isset($GLOBALS['TYPO3_LOADED_EXT'][$package->getPackageKey()])) {
-                       $loadedExtArrayElement = new \TYPO3\CMS\Core\Compatibility\LoadedExtensionArrayElement($package);
+                       $loadedExtArrayElement = new LoadedExtensionArrayElement($package);
                        $GLOBALS['TYPO3_LOADED_EXT'][$package->getPackageKey()] = $loadedExtArrayElement->toArray();
                }
        }
 
        /**
-        * removes a package from the file system.
+        * Removes a package from the file system.
         *
         * @param string $packageKey
         * @throws Exception
@@ -720,7 +729,7 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
                $packagePath = $package->getPackagePath();
                $deletion = GeneralUtility::rmdir($packagePath, TRUE);
                if ($deletion === FALSE) {
-                       throw new Exception('Please check file permissions. The directory "' . $packagePath . '" for package "' . $packageKey . '" could not be removed.', 1301491089, $exception);
+                       throw new Exception('Please check file permissions. The directory "' . $packagePath . '" for package "' . $packageKey . '" could not be removed.', 1301491089);
                }
        }
 
@@ -800,7 +809,7 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
         * Saves the current content of $this->packageStatesConfiguration to the
         * PackageStates.php file.
         *
-        * @return void
+        * @throws Exception\PackageStatesFileNotWritableException
         */
        protected function sortAndSavePackageStates() {
                $this->sortAvailablePackagesByDependencies();
@@ -883,7 +892,7 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
         * @param array $collectedPackagePaths
         * @return array
         */
-       protected function scanPackagesInPath($startPath, array &$collectedPackagePaths = array()) {
+       protected function scanPackagesInPath($startPath, array $collectedPackagePaths) {
                foreach (new \DirectoryIterator($startPath) as $fileInfo) {
                        if (!$fileInfo->isDir()) {
                                continue;
@@ -891,8 +900,9 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
                        $filename = $fileInfo->getFilename();
                        if ($filename[0] !== '.') {
                                $currentPath = $fileInfo->getPathName();
+                               $currentPath = PathUtility::sanitizeTrailingSeparator($currentPath);
                                if ($this->hasComposerManifestFile($currentPath)) {
-                                       $collectedPackagePaths[$currentPath . '/'] = $currentPath . '/';
+                                       $collectedPackagePaths[$currentPath] = $currentPath;
                                }
                        }
                }
@@ -900,34 +910,19 @@ class PackageManager implements \TYPO3\CMS\Core\SingletonInterface {
        }
 
        /**
-        * Returns contents of Composer manifest - or part there of.
+        * Returns contents of Composer manifest as a stdObject
         *
         * @param string $manifestPath
-        * @param string $key Optional. Only return the part of the manifest indexed by 'key'
-        * @param object $composerManifest Optional. Manifest to use instead of reading it from file
         * @return mixed
         * @throws Exception\MissingPackageManifestException
         * @see json_decode for return values
         */
-       static public function getComposerManifest($manifestPath, $key = NULL, $composerManifest = NULL) {
-               if ($composerManifest === NULL) {
-                       if (!file_exists($manifestPath . 'composer.json')) {
-                               throw new Exception\MissingPackageManifestException('No composer manifest file found at "' . $manifestPath . '/composer.json".', 1349868540);
-                       }
-                       $json = file_get_contents($manifestPath . 'composer.json');
-                       $composerManifest = json_decode($json);
-               }
-
-               if ($key !== NULL) {
-                       if (isset($composerManifest->{$key})) {
-                               $value = $composerManifest->{$key};
-                       } else {
-                               $value = NULL;
-                       }
-               } else {
-                       $value = $composerManifest;
+       public function getComposerManifest($manifestPath) {
+               if (!file_exists($manifestPath . 'composer.json')) {
+                       throw new Exception\MissingPackageManifestException('No composer manifest file found at "' . $manifestPath . '/composer.json".', 1349868540);
                }
-               return $value;
+               $json = file_get_contents($manifestPath . 'composer.json');
+               return json_decode($json);
        }
 
        /**
index 3594ab6..d6a1ac8 100644 (file)
@@ -108,7 +108,7 @@ class PackageTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
        public function getClassesPathReturnsPathToClasses() {
                $packageManagerMock = $this->getMock(\TYPO3\CMS\Core\Package\PackageManager::class);
                $packageManagerMock->expects($this->any())->method('isPackageKeyValid')->willReturn(TRUE);
-               $package = new Package($packageManagerMock, 'core', PATH_typo3 . 'sysext/core/', Package::DIRECTORY_CLASSES);
+               $package = new Package($packageManagerMock, 'core', PATH_typo3 . 'sysext/core/');
                $packageClassesPath = $package->getClassesPath();
                $expected = $package->getPackagePath() . Package::DIRECTORY_CLASSES;
                $this->assertEquals($expected, $packageClassesPath);