From 5868b0e0910adcf886d018f282e7d14c29a2b60c Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 28 Mar 2024 22:42:18 +0100 Subject: [PATCH 01/13] FEATURE: Introduce `FlowPackageKey` --- Neos.Flow/Classes/Package/FlowPackageKey.php | 44 +++++++++++++++++++ .../Classes/Package/PackageInterface.php | 3 +- 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 Neos.Flow/Classes/Package/FlowPackageKey.php diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php new file mode 100644 index 0000000000..0dd8eb2847 --- /dev/null +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -0,0 +1,44 @@ +value; + } +} diff --git a/Neos.Flow/Classes/Package/PackageInterface.php b/Neos.Flow/Classes/Package/PackageInterface.php index 5c7e115480..09c3550812 100644 --- a/Neos.Flow/Classes/Package/PackageInterface.php +++ b/Neos.Flow/Classes/Package/PackageInterface.php @@ -18,7 +18,8 @@ */ interface PackageInterface { - const PATTERN_MATCH_PACKAGEKEY = '/^[a-z0-9]+\.(?:[a-z0-9][\.a-z0-9]*)+$/i'; + /** @deprecated please use the FlowPackageKey instead */ + const PATTERN_MATCH_PACKAGEKEY = FlowPackageKey::PATTERN; const DEFAULT_COMPOSER_TYPE = 'neos-package'; /** From cb0bff8a1c568183ea06607b090e0af8aba0ceac Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 28 Mar 2024 22:44:02 +0100 Subject: [PATCH 02/13] WIP: Move package key determination logic to `FlowPackageKey` --- Neos.Flow/Classes/Package/FlowPackageKey.php | 75 +++++++++++++++++++ Neos.Flow/Classes/Package/PackageManager.php | 76 +------------------- 2 files changed, 76 insertions(+), 75 deletions(-) diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index 0dd8eb2847..310fb7cd47 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -5,6 +5,7 @@ namespace Neos\Flow\Package; use Neos\Flow\Annotations as Flow; +use Neos\Flow\Composer\ComposerUtility; final readonly class FlowPackageKey implements \JsonSerializable { @@ -37,6 +38,80 @@ public static function isPackageKeyValid(string $string): bool return preg_match(self::PATTERN, $string) === 1; } + /** + * Resolves package key from Composer manifest + * + * If it is a Flow package the name of the containing directory will be used. + * + * Else if the composer name of the package matches the first part of the lowercased namespace of the package, the mixed + * case version of the composer name / namespace will be used, with backslashes replaced by dots. + * + * Else the composer name will be used with the slash replaced by a dot + * + * @param array $manifest + * @param string $packagePath + * @return string + */ + protected function getPackageKeyFromManifest(array $manifest, $packagePath): string + { + if (isset($manifest['extra']['neos']['package-key']) && $this->isPackageKeyValid($manifest['extra']['neos']['package-key'])) { + return $manifest['extra']['neos']['package-key']; + } + + $composerName = $manifest['name']; + $autoloadNamespace = null; + $type = null; + if (isset($manifest['autoload']['psr-0']) && is_array($manifest['autoload']['psr-0'])) { + $namespaces = array_keys($manifest['autoload']['psr-0']); + $autoloadNamespace = reset($namespaces); + } + + if (isset($manifest['type'])) { + $type = $manifest['type']; + } + + return $this->derivePackageKey($composerName, $type, $packagePath, $autoloadNamespace); + } + + /** + * Derive a flow package key from the given information. + * The order of importance is: + * + * - package install path + * - first found autoload namespace + * - composer name + * + * @param string $composerName + * @param string $packageType + * @param string $packagePath + * @param string $autoloadNamespace + * @return string + */ + protected function derivePackageKey(string $composerName, string $packageType = null, string $packagePath = '', string $autoloadNamespace = null): string + { + $packageKey = ''; + + if ($packageType !== null && ComposerUtility::isFlowPackageType($packageType)) { + $lastSegmentOfPackagePath = substr(trim($packagePath, '/'), strrpos(trim($packagePath, '/'), '/') + 1); + if (strpos($lastSegmentOfPackagePath, '.') !== false) { + $packageKey = $lastSegmentOfPackagePath; + } + } + + if ($autoloadNamespace !== null && ($packageKey === null || $this->isPackageKeyValid($packageKey) === false)) { + $packageKey = str_replace('\\', '.', $autoloadNamespace); + } + + if ($packageKey === null || $this->isPackageKeyValid($packageKey) === false) { + $packageKey = str_replace('/', '.', $composerName); + } + + $packageKey = trim($packageKey, '.'); + $packageKey = preg_replace('/[^A-Za-z0-9.]/', '', $packageKey); + + return $packageKey; + } + public function jsonSerialize(): string { return $this->value; diff --git a/Neos.Flow/Classes/Package/PackageManager.php b/Neos.Flow/Classes/Package/PackageManager.php index f724e10179..bbfc61a9e3 100644 --- a/Neos.Flow/Classes/Package/PackageManager.php +++ b/Neos.Flow/Classes/Package/PackageManager.php @@ -829,81 +829,7 @@ public function getPackageKeyFromComposerName($composerName): string */ public function isPackageKeyValid($packageKey): bool { - return preg_match(PackageInterface::PATTERN_MATCH_PACKAGEKEY, $packageKey) === 1; - } - - /** - * Resolves package key from Composer manifest - * - * If it is a Flow package the name of the containing directory will be used. - * - * Else if the composer name of the package matches the first part of the lowercased namespace of the package, the mixed - * case version of the composer name / namespace will be used, with backslashes replaced by dots. - * - * Else the composer name will be used with the slash replaced by a dot - * - * @param array $manifest - * @param string $packagePath - * @return string - */ - protected function getPackageKeyFromManifest(array $manifest, $packagePath): string - { - if (isset($manifest['extra']['neos']['package-key']) && $this->isPackageKeyValid($manifest['extra']['neos']['package-key'])) { - return $manifest['extra']['neos']['package-key']; - } - - $composerName = $manifest['name']; - $autoloadNamespace = null; - $type = null; - if (isset($manifest['autoload']['psr-0']) && is_array($manifest['autoload']['psr-0'])) { - $namespaces = array_keys($manifest['autoload']['psr-0']); - $autoloadNamespace = reset($namespaces); - } - - if (isset($manifest['type'])) { - $type = $manifest['type']; - } - - return $this->derivePackageKey($composerName, $type, $packagePath, $autoloadNamespace); - } - - /** - * Derive a flow package key from the given information. - * The order of importance is: - * - * - package install path - * - first found autoload namespace - * - composer name - * - * @param string $composerName - * @param string $packageType - * @param string $packagePath - * @param string $autoloadNamespace - * @return string - */ - protected function derivePackageKey(string $composerName, string $packageType = null, string $packagePath = '', string $autoloadNamespace = null): string - { - $packageKey = ''; - - if ($packageType !== null && ComposerUtility::isFlowPackageType($packageType)) { - $lastSegmentOfPackagePath = substr(trim($packagePath, '/'), strrpos(trim($packagePath, '/'), '/') + 1); - if (strpos($lastSegmentOfPackagePath, '.') !== false) { - $packageKey = $lastSegmentOfPackagePath; - } - } - - if ($autoloadNamespace !== null && ($packageKey === null || $this->isPackageKeyValid($packageKey) === false)) { - $packageKey = str_replace('\\', '.', $autoloadNamespace); - } - - if ($packageKey === null || $this->isPackageKeyValid($packageKey) === false) { - $packageKey = str_replace('/', '.', $composerName); - } - - $packageKey = trim($packageKey, '.'); - $packageKey = preg_replace('/[^A-Za-z0-9.]/', '', $packageKey); - - return $packageKey; + return FlowPackageKey::isPackageKeyValid($packageKey); } /** From 92486d78caa8fcb2a82d47539f5ed322f59a9f99 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 28 Mar 2024 22:50:05 +0100 Subject: [PATCH 03/13] TASK: Fix package key determination logic in `FlowPackageKey` --- Neos.Flow/Classes/Package/FlowPackageKey.php | 28 +++++++------------- Neos.Flow/Classes/Package/PackageManager.php | 6 ++--- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index 310fb7cd47..d673bad4f6 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -47,15 +47,13 @@ public static function isPackageKeyValid(string $string): bool * case version of the composer name / namespace will be used, with backslashes replaced by dots. * * Else the composer name will be used with the slash replaced by a dot - * - * @param array $manifest - * @param string $packagePath - * @return string */ - protected function getPackageKeyFromManifest(array $manifest, $packagePath): string + public static function getPackageKeyFromManifest(array $manifest, string $packagePath): self { - if (isset($manifest['extra']['neos']['package-key']) && $this->isPackageKeyValid($manifest['extra']['neos']['package-key'])) { - return $manifest['extra']['neos']['package-key']; + $definedFlowPackageKey = $manifest['extra']['neos']['package-key'] ?? null; + + if ($definedFlowPackageKey && self::isPackageKeyValid($definedFlowPackageKey)) { + return new self($definedFlowPackageKey); } $composerName = $manifest['name']; @@ -70,7 +68,7 @@ protected function getPackageKeyFromManifest(array $manifest, $packagePath): str $type = $manifest['type']; } - return $this->derivePackageKey($composerName, $type, $packagePath, $autoloadNamespace); + return self::derivePackageKey($composerName, $type, $packagePath, $autoloadNamespace); } /** @@ -80,14 +78,8 @@ protected function getPackageKeyFromManifest(array $manifest, $packagePath): str * - package install path * - first found autoload namespace * - composer name - * - * @param string $composerName - * @param string $packageType - * @param string $packagePath - * @param string $autoloadNamespace - * @return string */ - protected function derivePackageKey(string $composerName, string $packageType = null, string $packagePath = '', string $autoloadNamespace = null): string + private static function derivePackageKey(string $composerName, string $packageType = null, string $packagePath = '', string $autoloadNamespace = null): self { $packageKey = ''; @@ -98,18 +90,18 @@ protected function derivePackageKey(string $composerName, string $packageType = } } - if ($autoloadNamespace !== null && ($packageKey === null || $this->isPackageKeyValid($packageKey) === false)) { + if ($autoloadNamespace !== null && ($packageKey === null || self::isPackageKeyValid($packageKey) === false)) { $packageKey = str_replace('\\', '.', $autoloadNamespace); } - if ($packageKey === null || $this->isPackageKeyValid($packageKey) === false) { + if ($packageKey === null || self::isPackageKeyValid($packageKey) === false) { $packageKey = str_replace('/', '.', $composerName); } $packageKey = trim($packageKey, '.'); $packageKey = preg_replace('/[^A-Za-z0-9.]/', '', $packageKey); - return $packageKey; + return new self($packageKey); } public function jsonSerialize(): string diff --git a/Neos.Flow/Classes/Package/PackageManager.php b/Neos.Flow/Classes/Package/PackageManager.php index bbfc61a9e3..34425f883a 100644 --- a/Neos.Flow/Classes/Package/PackageManager.php +++ b/Neos.Flow/Classes/Package/PackageManager.php @@ -592,10 +592,10 @@ protected function scanAvailablePackages(): array continue; } - $packageKey = $this->getPackageKeyFromManifest($composerManifest, $packagePath); - $this->composerNameToPackageKeyMap[strtolower($composerManifest['name'])] = $packageKey; + $packageKey = FlowPackageKey::getPackageKeyFromManifest($composerManifest, $packagePath); + $this->composerNameToPackageKeyMap[strtolower($composerManifest['name'])] = $packageKey->value; - $packageConfiguration = $this->preparePackageStateConfiguration($packageKey, $packagePath, $composerManifest); + $packageConfiguration = $this->preparePackageStateConfiguration($packageKey->value, $packagePath, $composerManifest); if (isset($newPackageStatesConfiguration['packages'][$composerManifest['name']])) { throw new PackageException( sprintf( From 7a26c3cf143e0cc4ab6ee89e19fa1e53d9c82b16 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 28 Mar 2024 22:54:34 +0100 Subject: [PATCH 04/13] TASK: Adjust code-style in `FlowPackageKey` and remove obsolete code --- Neos.Flow/Classes/Package/FlowPackageKey.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index d673bad4f6..329e415669 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -79,22 +79,22 @@ public static function getPackageKeyFromManifest(array $manifest, string $packag * - first found autoload namespace * - composer name */ - private static function derivePackageKey(string $composerName, string $packageType = null, string $packagePath = '', string $autoloadNamespace = null): self + private static function derivePackageKey(string $composerName, ?string $packageType, string $packagePath, ?string $autoloadNamespace): self { $packageKey = ''; if ($packageType !== null && ComposerUtility::isFlowPackageType($packageType)) { $lastSegmentOfPackagePath = substr(trim($packagePath, '/'), strrpos(trim($packagePath, '/'), '/') + 1); - if (strpos($lastSegmentOfPackagePath, '.') !== false) { + if (str_contains($lastSegmentOfPackagePath, '.')) { $packageKey = $lastSegmentOfPackagePath; } } - if ($autoloadNamespace !== null && ($packageKey === null || self::isPackageKeyValid($packageKey) === false)) { + if ($autoloadNamespace !== null && (self::isPackageKeyValid($packageKey) === false)) { $packageKey = str_replace('\\', '.', $autoloadNamespace); } - if ($packageKey === null || self::isPackageKeyValid($packageKey) === false) { + if (self::isPackageKeyValid($packageKey) === false) { $packageKey = str_replace('/', '.', $composerName); } From e82285bae89fbcfa5d641c0fa180182ef2728dfd Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 28 Mar 2024 23:32:07 +0100 Subject: [PATCH 05/13] TASK: Use `FlowPackageKey` internally in PackageFactory --- Neos.Flow/Classes/Package/PackageFactory.php | 26 +++++++++---------- Neos.Flow/Classes/Package/PackageManager.php | 12 +++------ .../Tests/Unit/Package/PackageFactoryTest.php | 13 +++++----- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/Neos.Flow/Classes/Package/PackageFactory.php b/Neos.Flow/Classes/Package/PackageFactory.php index 8a3725c352..b9f85d4b79 100644 --- a/Neos.Flow/Classes/Package/PackageFactory.php +++ b/Neos.Flow/Classes/Package/PackageFactory.php @@ -25,14 +25,14 @@ class PackageFactory * * @param string $packagesBasePath the base install path of packages, * @param string $packagePath path to package, relative to base path - * @param string $packageKey key / name of the package + * @param FlowPackageKey $packageKey key / name of the package * @param string $composerName * @param array $autoloadConfiguration Autoload configuration as defined in composer.json - * @param array $packageClassInformation - * @return PackageInterface|PackageKeyAwareInterface + * @param array{className: class-string, pathAndFilename: string}|null $packageClassInformation + * @return PackageInterface&PackageKeyAwareInterface * @throws Exception\CorruptPackageException */ - public function create($packagesBasePath, $packagePath, $packageKey, $composerName, array $autoloadConfiguration = [], array $packageClassInformation = null) + public function create(string $packagesBasePath, string $packagePath, FlowPackageKey $packageKey, string $composerName, array $autoloadConfiguration = [], array $packageClassInformation = null): PackageInterface { $absolutePackagePath = Files::concatenatePaths([$packagesBasePath, $packagePath]) . '/'; @@ -50,24 +50,24 @@ public function create($packagesBasePath, $packagePath, $packageKey, $composerNa require_once($packageClassPath); } - $package = new $packageClassName($packageKey, $composerName, $absolutePackagePath, $autoloadConfiguration); + /** dynamic construction {@see GenericPackage::__construct} */ + $package = new $packageClassName($packageKey->value, $composerName, $absolutePackagePath, $autoloadConfiguration); if (!$package instanceof PackageInterface) { - throw new Exception\CorruptPackageException(sprintf('The package class of package "%s" does not implement \Neos\Flow\Package\PackageInterface. Check the file "%s".', $packageKey, $packageClassInformation['pathAndFilename']), 1427193370); + throw new Exception\CorruptPackageException(sprintf('The package class of package "%s" does not implement \Neos\Flow\Package\PackageInterface. Check the file "%s".', $packageKey->value, $packageClassInformation['pathAndFilename']), 1427193370); } - return $package; } /** * Detects if the package contains a package file and returns the path and classname. * - * @param string $packageKey The package key + * @param FlowPackageKey $packageKey The package key * @param string $absolutePackagePath Absolute path to the package - * @return array The path to the package file and classname for this package or an empty array if none was found. + * @return array{className: class-string, pathAndFilename: string} The path to the package file and classname for this package or an empty array if none was found. * @throws Exception\CorruptPackageException * @throws Exception\InvalidPackagePathException */ - public function detectFlowPackageFilePath($packageKey, $absolutePackagePath) + public function detectFlowPackageFilePath(FlowPackageKey $packageKey, $absolutePackagePath): array { if (!is_dir($absolutePackagePath)) { throw new Exception\InvalidPackagePathException(sprintf('The given package path "%s" is not a readable directory.', $absolutePackagePath), 1445904440); @@ -80,7 +80,7 @@ public function detectFlowPackageFilePath($packageKey, $absolutePackagePath) $possiblePackageClassPaths = [ Files::concatenatePaths(['Classes', 'Package.php']), - Files::concatenatePaths(['Classes', str_replace('.', '/', $packageKey), 'Package.php']) + Files::concatenatePaths(['Classes', str_replace('.', '/', $packageKey->value), 'Package.php']) ]; $foundPackageClassPaths = array_filter($possiblePackageClassPaths, function ($packageClassPathAndFilename) use ($absolutePackagePath) { @@ -93,7 +93,7 @@ public function detectFlowPackageFilePath($packageKey, $absolutePackagePath) } if (count($foundPackageClassPaths) > 1) { - throw new Exception\CorruptPackageException(sprintf('The package "%s" contains multiple possible "Package.php" files. Please make sure that only one "Package.php" exists in the autoload root(s) of your Flow package.', $packageKey), 1457454840); + throw new Exception\CorruptPackageException(sprintf('The package "%s" contains multiple possible "Package.php" files. Please make sure that only one "Package.php" exists in the autoload root(s) of your Flow package.', $packageKey->value), 1457454840); } $packageClassPathAndFilename = reset($foundPackageClassPaths); @@ -102,7 +102,7 @@ public function detectFlowPackageFilePath($packageKey, $absolutePackagePath) $packageClassContents = file_get_contents($absolutePackageClassPath); $packageClassName = (new PhpAnalyzer($packageClassContents))->extractFullyQualifiedClassName(); if ($packageClassName === null) { - throw new Exception\CorruptPackageException(sprintf('The package "%s" does not contain a valid package class. Check if the file "%s" really contains a class.', $packageKey, $packageClassPathAndFilename), 1327587091); + throw new Exception\CorruptPackageException(sprintf('The package "%s" does not contain a valid package class. Check if the file "%s" really contains a class.', $packageKey->value, $packageClassPathAndFilename), 1327587091); } return ['className' => $packageClassName, 'pathAndFilename' => $packageClassPathAndFilename]; diff --git a/Neos.Flow/Classes/Package/PackageManager.php b/Neos.Flow/Classes/Package/PackageManager.php index 34425f883a..3efa19387c 100644 --- a/Neos.Flow/Classes/Package/PackageManager.php +++ b/Neos.Flow/Classes/Package/PackageManager.php @@ -595,7 +595,7 @@ protected function scanAvailablePackages(): array $packageKey = FlowPackageKey::getPackageKeyFromManifest($composerManifest, $packagePath); $this->composerNameToPackageKeyMap[strtolower($composerManifest['name'])] = $packageKey->value; - $packageConfiguration = $this->preparePackageStateConfiguration($packageKey->value, $packagePath, $composerManifest); + $packageConfiguration = $this->preparePackageStateConfiguration($packageKey, $packagePath, $composerManifest); if (isset($newPackageStatesConfiguration['packages'][$composerManifest['name']])) { throw new PackageException( sprintf( @@ -640,19 +640,15 @@ protected function findComposerPackagesInPath(string $startingDirectory): \Gener } /** - * @param string $packageKey - * @param string $packagePath - * @param array $composerManifest - * @return array * @throws Exception\CorruptPackageException * @throws Exception\InvalidPackagePathException */ - protected function preparePackageStateConfiguration($packageKey, $packagePath, $composerManifest): array + protected function preparePackageStateConfiguration(FlowPackageKey $packageKey, string $packagePath, array $composerManifest): array { $autoload = $composerManifest['autoload'] ?? []; return [ - 'packageKey' => $packageKey, + 'packageKey' => $packageKey->value, 'packagePath' => str_replace($this->packagesBasePath, '', $packagePath), 'composerName' => $composerManifest['name'], 'autoloadConfiguration' => $autoload, @@ -687,7 +683,7 @@ protected function registerPackageFromStateConfiguration($composerName, $package { $packagePath = $packageStateConfiguration['packagePath'] ?? null; $packageClassInformation = $packageStateConfiguration['packageClassInformation'] ?? null; - $package = $this->packageFactory->create($this->packagesBasePath, $packagePath, $packageStateConfiguration['packageKey'], $composerName, $packageStateConfiguration['autoloadConfiguration'], $packageClassInformation); + $package = $this->packageFactory->create($this->packagesBasePath, $packagePath, FlowPackageKey::fromString($packageStateConfiguration['packageKey']), $composerName, $packageStateConfiguration['autoloadConfiguration'], $packageClassInformation); $this->packageKeys[strtolower($package->getPackageKey())] = $package->getPackageKey(); $this->packages[$package->getPackageKey()] = $package; } diff --git a/Neos.Flow/Tests/Unit/Package/PackageFactoryTest.php b/Neos.Flow/Tests/Unit/Package/PackageFactoryTest.php index 01c3eaa55f..8109b55985 100644 --- a/Neos.Flow/Tests/Unit/Package/PackageFactoryTest.php +++ b/Neos.Flow/Tests/Unit/Package/PackageFactoryTest.php @@ -13,6 +13,7 @@ use Neos\Flow\Package\Exception\CorruptPackageException; use Neos\Flow\Package\Exception\InvalidPackagePathException; +use Neos\Flow\Package\FlowPackageKey; use org\bovigo\vfs\vfsStream; use Neos\Flow\Composer\ComposerUtility; use Neos\Flow\Package\Package; @@ -54,7 +55,7 @@ protected function setUp(): void public function createThrowsExceptionWhenSpecifyingANonExistingPackagePath() { $this->expectException(InvalidPackagePathException::class); - $this->packageFactory->create('vfs://Packages/', 'Some/Non/Existing/Path/Some.Package/', 'Some.Package', 'some/package'); + $this->packageFactory->create('vfs://Packages/', 'Some/Non/Existing/Path/Some.Package/', FlowPackageKey::fromString('Some.Package'), 'some/package'); } /** @@ -69,7 +70,7 @@ public function createThrowsExceptionIfCustomPackageFileCantBeAnalyzed() file_put_contents($packagePath . 'composer.json', '{"name": "some/package", "type": "neos-test", "autoload": { "psr-0": { "Foo": "bar" }}}'); file_put_contents($packageFilePath, 'packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', 'Some.Package', 'some/package'); + $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', FlowPackageKey::fromString('Some.Package'), 'some/package'); } /** @@ -86,7 +87,7 @@ public function createThrowsExceptionIfCustomPackageDoesNotImplementPackageInter require($packageFilePath); - $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', 'Some.Package', 'some/package'); + $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', FlowPackageKey::fromString('Some.Package'), 'some/package'); } /** @@ -102,7 +103,7 @@ public function createReturnsInstanceOfCustomPackageIfItExists() require($packageFilePath); - $package = $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', 'Some.Package', 'some/package'); + $package = $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', FlowPackageKey::fromString('Some.Package'), 'some/package'); self::assertSame('Neos\Flow\Fixtures\CustomPackage2', get_class($package)); } @@ -121,7 +122,7 @@ public function createTakesAutoloaderTypeIntoAccountWhenLoadingCustomPackage() require($packageFilePath); - $package = $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', 'Some.Package', 'some/package', $composerManifest['autoload']); + $package = $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', FlowPackageKey::fromString('Some.Package'), 'some/package', $composerManifest['autoload']); self::assertSame('Neos\Flow\Fixtures\CustomPackage3', get_class($package)); } @@ -134,7 +135,7 @@ public function createReturnsAnInstanceOfTheDefaultPackageIfNoCustomPackageExist mkdir($packagePath, 0777, true); file_put_contents($packagePath . 'composer.json', '{"name": "some/package", "type": "neos-test"}'); - $package = $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', 'Some.Package', 'some/package'); + $package = $this->packageFactory->create('vfs://Packages/', 'Some/Path/Some.Package/', FlowPackageKey::fromString('Some.Package'), 'some/package'); self::assertSame(Package::class, get_class($package)); } } From 4b429402c5cb66864b0ea101ab2b7611a69b762b Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 28 Mar 2024 23:36:00 +0100 Subject: [PATCH 06/13] TASK: Make phpstan happy. - Custom Package.php must always implement the `PackageKeyAwareInterface` (which they do by extending the Flow Package) - $packageClassInformation will never be empty at this point as `detectFlowPackageFilePath` is guaranteed to return or will throw. --- Neos.Flow/Classes/Package/PackageFactory.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Neos.Flow/Classes/Package/PackageFactory.php b/Neos.Flow/Classes/Package/PackageFactory.php index b9f85d4b79..e3856cf238 100644 --- a/Neos.Flow/Classes/Package/PackageFactory.php +++ b/Neos.Flow/Classes/Package/PackageFactory.php @@ -40,13 +40,10 @@ public function create(string $packagesBasePath, string $packagePath, FlowPackag $packageClassInformation = $this->detectFlowPackageFilePath($packageKey, $absolutePackagePath); } - $packageClassName = Package::class; - if (!empty($packageClassInformation)) { - $packageClassName = $packageClassInformation['className']; - $packageClassPath = !empty($packageClassInformation['pathAndFilename']) ? Files::concatenatePaths([$absolutePackagePath, $packageClassInformation['pathAndFilename']]) : null; - } + $packageClassName = $packageClassInformation['className']; - if (!empty($packageClassPath)) { + if (!empty($packageClassInformation['pathAndFilename'])) { + $packageClassPath = Files::concatenatePaths([$absolutePackagePath, $packageClassInformation['pathAndFilename']]); require_once($packageClassPath); } @@ -55,6 +52,9 @@ public function create(string $packagesBasePath, string $packagePath, FlowPackag if (!$package instanceof PackageInterface) { throw new Exception\CorruptPackageException(sprintf('The package class of package "%s" does not implement \Neos\Flow\Package\PackageInterface. Check the file "%s".', $packageKey->value, $packageClassInformation['pathAndFilename']), 1427193370); } + if (!$package instanceof PackageKeyAwareInterface) { + throw new Exception\CorruptPackageException(sprintf('The package class of package "%s" does not implement \Neos\Flow\Package\PackageKeyAwareInterface. Check the file "%s".', $packageKey->value, $packageClassInformation['pathAndFilename']), 1711665156); + } return $package; } From fc70fed44e9c4713e28d380009726cb2950cfc31 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Thu, 28 Mar 2024 23:49:57 +0100 Subject: [PATCH 07/13] TASK: Introduce `FlowPackageKey` optionally to PackageManager api --- .../Classes/Composer/ComposerUtility.php | 16 ++++---- Neos.Flow/Classes/Package/FlowPackageKey.php | 2 +- Neos.Flow/Classes/Package/PackageManager.php | 37 ++++++++++--------- .../Tests/Unit/Package/PackageManagerTest.php | 16 ++++---- 4 files changed, 37 insertions(+), 34 deletions(-) diff --git a/Neos.Flow/Classes/Composer/ComposerUtility.php b/Neos.Flow/Classes/Composer/ComposerUtility.php index 94cca5a9ab..70c0401689 100644 --- a/Neos.Flow/Classes/Composer/ComposerUtility.php +++ b/Neos.Flow/Classes/Composer/ComposerUtility.php @@ -12,6 +12,7 @@ */ use Neos\Flow\Package\FlowPackageInterface; +use Neos\Flow\Package\FlowPackageKey; use Neos\Utility\ObjectAccess; use Neos\Utility\Files; @@ -127,13 +128,10 @@ public static function isFlowPackageType(string $packageType): bool /** * Determines the composer package name ("vendor/foo-bar") from the Flow package key ("Vendor.Foo.Bar") - * - * @param string $packageKey - * @return string */ - public static function getComposerPackageNameFromPackageKey(string $packageKey): string + public static function getComposerPackageNameFromPackageKey(FlowPackageKey $packageKey): string { - $nameParts = explode('.', $packageKey); + $nameParts = explode('.', $packageKey->value); $vendor = array_shift($nameParts); return strtolower($vendor . '/' . implode('-', $nameParts)); } @@ -142,11 +140,11 @@ public static function getComposerPackageNameFromPackageKey(string $packageKey): * Write a composer manifest for the package. * * @param string $manifestPath - * @param string $packageKey + * @param FlowPackageKey $packageKey * @param array $composerManifestData * @return array the manifest data written */ - public static function writeComposerManifest(string $manifestPath, string $packageKey, array $composerManifestData = []): array + public static function writeComposerManifest(string $manifestPath, FlowPackageKey $packageKey, array $composerManifestData = []): array { $manifest = [ 'description' => '' @@ -164,11 +162,11 @@ public static function writeComposerManifest(string $manifestPath, string $packa } if (!isset($manifest['autoload'])) { - $namespace = str_replace('.', '\\', $packageKey) . '\\'; + $namespace = str_replace('.', '\\', $packageKey->value) . '\\'; $manifest['autoload'] = ['psr-4' => [$namespace => FlowPackageInterface::DIRECTORY_CLASSES]]; } - $manifest['extra']['neos']['package-key'] = $packageKey; + $manifest['extra']['neos']['package-key'] = $packageKey->value; if (defined('JSON_PRETTY_PRINT')) { file_put_contents(Files::concatenatePaths([$manifestPath, 'composer.json']), json_encode($manifest, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT)); diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index 329e415669..5205ef803b 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -18,7 +18,7 @@ private function __construct( public string $value ) { if (!self::isPackageKeyValid($value)) { - throw new \InvalidArgumentException(sprintf('Not a valid Flow package key: "%s"', $value), 1711659821); + throw new Exception\InvalidPackageKeyException('The package key "' . $value . '" is invalid', 1220722210); } } diff --git a/Neos.Flow/Classes/Package/PackageManager.php b/Neos.Flow/Classes/Package/PackageManager.php index 3efa19387c..a9ea1d238d 100644 --- a/Neos.Flow/Classes/Package/PackageManager.php +++ b/Neos.Flow/Classes/Package/PackageManager.php @@ -68,7 +68,7 @@ class PackageManager /** * A translation table between lower cased and upper camel cased package keys * - * @var array + * @var array */ protected $packageKeys = []; @@ -175,11 +175,11 @@ public function getFlowPackages(): array * Returns true if a package is available (the package's files exist in the packages directory) * or false if it's not. * - * @param string $packageKey The key of the package to check + * @param string|FlowPackageKey $packageKey The key of the package to check * @return boolean true if the package is available, otherwise false * @api */ - public function isPackageAvailable($packageKey): bool + public function isPackageAvailable(string|FlowPackageKey $packageKey): bool { return ($this->getCaseSensitivePackageKey($packageKey) !== false); } @@ -197,15 +197,16 @@ public function getPackagesBasePath(): string /** * Returns a PackageInterface object for the specified package. * - * @param string $packageKey + * @param string|FlowPackageKey $packageKey * @return PackageInterface The requested package object * @throws Exception\UnknownPackageException if the specified package is not known * @api */ - public function getPackage($packageKey): PackageInterface + public function getPackage(string|FlowPackageKey $packageKey): PackageInterface { if (!$this->isPackageAvailable($packageKey)) { - throw new Exception\UnknownPackageException('Package "' . $packageKey . '" is not available. Please check if the package exists and that the package key is correct (package keys are case sensitive).', 1166546734); + $packageKeyString = $packageKey instanceof FlowPackageKey ? $packageKey->value : $packageKey; + throw new Exception\UnknownPackageException('Package "' . $packageKeyString . '" is not available. Please check if the package exists and that the package key is correct (package keys are case sensitive).', 1166546734); } return $this->packages[$this->getCaseSensitivePackageKey($packageKey)]; @@ -314,13 +315,13 @@ protected function filterPackagesByType($packages, $packageType): array * @throws InvalidConfigurationException * @api */ - public function createPackage($packageKey, array $manifest = [], $packagesPath = null): PackageInterface + public function createPackage(string|FlowPackageKey $packageKey, array $manifest = [], $packagesPath = null): PackageInterface { - if (!$this->isPackageKeyValid($packageKey)) { - throw new Exception\InvalidPackageKeyException('The package key "' . $packageKey . '" is invalid', 1220722210); + if (!$packageKey instanceof FlowPackageKey) { + $packageKey = FlowPackageKey::fromString($packageKey); } if ($this->isPackageAvailable($packageKey)) { - throw new Exception\PackageKeyAlreadyExistsException('The package key "' . $packageKey . '" already exists', 1220722873); + throw new Exception\PackageKeyAlreadyExistsException('The package key "' . $packageKey->value . '" already exists', 1220722873); } if (!isset($manifest['type'])) { $manifest['type'] = PackageInterface::DEFAULT_COMPOSER_TYPE; @@ -352,7 +353,7 @@ public function createPackage($packageKey, array $manifest = [], $packagesPath = $packagesPath = Files::getUnixStylePath(Files::concatenatePaths([$this->packagesBasePath, $packagesPath])); } - $packagePath = Files::concatenatePaths([$packagesPath, $packageKey]) . '/'; + $packagePath = Files::concatenatePaths([$packagesPath, $packageKey->value]) . '/'; Files::createDirectoryRecursively($packagePath); foreach ( @@ -388,9 +389,9 @@ public function createPackage($packageKey, array $manifest = [], $packagesPath = $refreshedPackageStatesConfiguration = $this->rescanPackages(); $this->packageStatesConfiguration = $refreshedPackageStatesConfiguration; $this->registerPackageFromStateConfiguration($manifest['name'], $this->packageStatesConfiguration['packages'][$manifest['name']]); - $package = $this->packages[$packageKey]; + $package = $this->packages[$packageKey->value]; if ($package instanceof FlowPackageInterface) { - $this->flowPackages[$packageKey] = $package; + $this->flowPackages[$packageKey->value] = $package; } return $package; @@ -782,13 +783,15 @@ protected function collectPackageManifestData(array $packageStates): array * Returns the correctly cased version of the given package key or false * if no such package is available. * - * @param string $unknownCasedPackageKey The package key to convert + * @param string|FlowPackageKey $unknownCasedPackageKey The package key to convert * @return string|false The upper camel cased package key or false if no such package exists * @api */ - public function getCaseSensitivePackageKey($unknownCasedPackageKey) + public function getCaseSensitivePackageKey(string|FlowPackageKey $unknownCasedPackageKey): string|false { - $lowerCasedPackageKey = strtolower($unknownCasedPackageKey); + $lowerCasedPackageKey = strtolower( + $unknownCasedPackageKey instanceof FlowPackageKey ? $unknownCasedPackageKey->value : $unknownCasedPackageKey + ); return $this->packageKeys[$lowerCasedPackageKey] ?? false; } @@ -821,7 +824,7 @@ public function getPackageKeyFromComposerName($composerName): string * * @param string $packageKey The package key to validate * @return boolean If the package key is valid, returns true otherwise false - * @api + * @deprecated with Flow 9.0 {@see FlowPackageKey::isPackageKeyValid()} */ public function isPackageKeyValid($packageKey): bool { diff --git a/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php b/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php index ab32cb06e3..70f7c45567 100644 --- a/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php +++ b/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php @@ -19,6 +19,7 @@ use Neos\Flow\Package\Exception\PackageKeyAlreadyExistsException; use Neos\Flow\Package\Exception\UnknownPackageException; use Neos\Flow\Package\FlowPackageInterface; +use Neos\Flow\Package\FlowPackageKey; use Neos\Flow\Package\PackageFactory; use Neos\Flow\Package\PackageInterface; use org\bovigo\vfs\vfsStream; @@ -234,14 +235,15 @@ public function scanAvailablePackagesTraversesThePackagesDirectoryAndRespectsPac */ public function packageStatesConfigurationContainsRelativePaths() { + /** @var list $packageKeys */ $packageKeys = [ - 'RobertLemke.Flow.NothingElse' . md5(uniqid(mt_rand(), true)), - 'Neos.Flow' . md5(uniqid(mt_rand(), true)), - 'Neos.YetAnotherTestPackage' . md5(uniqid(mt_rand(), true)), + FlowPackageKey::fromString('RobertLemke.Flow.NothingElse' . md5(uniqid(mt_rand(), true))), + FlowPackageKey::fromString('Neos.Flow' . md5(uniqid(mt_rand(), true))), + FlowPackageKey::fromString('Neos.YetAnotherTestPackage' . md5(uniqid(mt_rand(), true))), ]; foreach ($packageKeys as $packageKey) { - $packagePath = 'vfs://Test/Packages/Application/' . $packageKey . '/'; + $packagePath = 'vfs://Test/Packages/Application/' . $packageKey->value . '/'; mkdir($packagePath, 0770, true); mkdir($packagePath . 'Classes'); @@ -252,7 +254,7 @@ public function packageStatesConfigurationContainsRelativePaths() $packageManager->_set('packagesBasePath', 'vfs://Test/Packages/'); $packageManager->_set('packageInformationCacheFilePath', 'vfs://Test/Configuration/PackageStates.php'); - $packageFactory = new PackageFactory($packageManager); + $packageFactory = new PackageFactory(); $this->inject($packageManager, 'packageFactory', $packageFactory); $packageManager->_set('packages', []); @@ -262,10 +264,10 @@ public function packageStatesConfigurationContainsRelativePaths() foreach ($packageKeys as $packageKey) { $composerName = ComposerUtility::getComposerPackageNameFromPackageKey($packageKey); $expectedPackageStatesConfiguration[$composerName] = [ - 'packagePath' => 'Application/' . $packageKey . '/', + 'packagePath' => 'Application/' . $packageKey->value . '/', 'composerName' => $composerName, 'packageClassInformation' => ['className' => 'Neos\Flow\Package\GenericPackage', 'pathAndFilename' => ''], - 'packageKey' => $packageKey, + 'packageKey' => $packageKey->value, 'autoloadConfiguration' => [] ]; } From 249056726446a6fc6065f23def92475d062e8baf Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 29 Mar 2024 00:02:11 +0100 Subject: [PATCH 08/13] TASK: Move `getComposerPackageNameFromPackageKey` to `FlowPackageKey::guessComposerPackageName` --- Neos.Flow/Classes/Composer/ComposerUtility.php | 14 ++------------ Neos.Flow/Classes/Package/FlowPackageKey.php | 10 ++++++++++ .../Tests/Unit/Package/PackageManagerTest.php | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Neos.Flow/Classes/Composer/ComposerUtility.php b/Neos.Flow/Classes/Composer/ComposerUtility.php index 70c0401689..5831dca541 100644 --- a/Neos.Flow/Classes/Composer/ComposerUtility.php +++ b/Neos.Flow/Classes/Composer/ComposerUtility.php @@ -19,7 +19,7 @@ /** * Utility to access composer information like composer manifests (composer.json) and the lock file. * - * Meant to be used only inside the Flow package management code. + * @internal Meant to be used only inside the Flow package management code. */ class ComposerUtility { @@ -126,16 +126,6 @@ public static function isFlowPackageType(string $packageType): bool return false; } - /** - * Determines the composer package name ("vendor/foo-bar") from the Flow package key ("Vendor.Foo.Bar") - */ - public static function getComposerPackageNameFromPackageKey(FlowPackageKey $packageKey): string - { - $nameParts = explode('.', $packageKey->value); - $vendor = array_shift($nameParts); - return strtolower($vendor . '/' . implode('-', $nameParts)); - } - /** * Write a composer manifest for the package. * @@ -154,7 +144,7 @@ public static function writeComposerManifest(string $manifestPath, FlowPackageKe $manifest = array_merge($manifest, $composerManifestData); } if (!isset($manifest['name']) || empty($manifest['name'])) { - $manifest['name'] = static::getComposerPackageNameFromPackageKey($packageKey); + $manifest['name'] = $packageKey->guessComposerPackageName(); } if (!isset($manifest['require']) || empty($manifest['require'])) { diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index 5205ef803b..fa943604b9 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -104,6 +104,16 @@ private static function derivePackageKey(string $composerName, ?string $packageT return new self($packageKey); } + /** + * Determines the composer package name ("vendor/foo-bar") from the Flow package key ("Vendor.Foo.Bar") + */ + public function guessComposerPackageName(): string + { + $nameParts = explode('.', $this->value); + $vendor = array_shift($nameParts); + return strtolower($vendor . '/' . implode('-', $nameParts)); + } + public function jsonSerialize(): string { return $this->value; diff --git a/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php b/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php index 70f7c45567..017d2ed6da 100644 --- a/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php +++ b/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php @@ -262,7 +262,7 @@ public function packageStatesConfigurationContainsRelativePaths() $expectedPackageStatesConfiguration = []; foreach ($packageKeys as $packageKey) { - $composerName = ComposerUtility::getComposerPackageNameFromPackageKey($packageKey); + $composerName = $packageKey->guessComposerPackageName(); $expectedPackageStatesConfiguration[$composerName] = [ 'packagePath' => 'Application/' . $packageKey->value . '/', 'composerName' => $composerName, From 8965695c323d9025f72a013ac46ffc6e68cfb283 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 29 Mar 2024 00:03:35 +0100 Subject: [PATCH 09/13] TASK: Document `FlowPackageKey` --- Neos.Flow/Classes/Package/FlowPackageKey.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index fa943604b9..0754588947 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -7,6 +7,21 @@ use Neos\Flow\Annotations as Flow; use Neos\Flow\Composer\ComposerUtility; +/** + * (Legacy) Flow representation of a package key. + * + * With the rise of composer each package _has_ a key like "vendor/foo-bar". + * But before the adaption Flow already established the package keys like "Vendor.Foo.Bar", + * which is represented and validated by this value object. + * + * The Flow package keys are currently inferred from the composer manifest {@see FlowPackageKey::getPackageKeyFromManifest()}, + * and can also be tried to be reverse calculated: {@see FlowPackageKey::guessComposerPackageName()} + * + * The idea around the Flow package key is obsolete since composer and will eventually be replaced. + * Still major parts of Flow depend on the concept. + * + * @internal Only meant to be used inside the Flow core until replaced by composer keys. + */ final readonly class FlowPackageKey implements \JsonSerializable { public const PATTERN = '/^[a-z0-9]+\.(?:[a-z0-9][\.a-z0-9]*)+$/i'; From 673fa74199838c2e99f9a290c7ea922b952d2c90 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 29 Mar 2024 11:41:28 +0100 Subject: [PATCH 10/13] TASK: Revert `FlowPackageKey` in package manager API --- .../Classes/Composer/ComposerUtility.php | 2 +- Neos.Flow/Classes/Package/FlowPackageKey.php | 2 +- Neos.Flow/Classes/Package/PackageManager.php | 29 ++++++++----------- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/Neos.Flow/Classes/Composer/ComposerUtility.php b/Neos.Flow/Classes/Composer/ComposerUtility.php index 5831dca541..b729a46693 100644 --- a/Neos.Flow/Classes/Composer/ComposerUtility.php +++ b/Neos.Flow/Classes/Composer/ComposerUtility.php @@ -19,7 +19,7 @@ /** * Utility to access composer information like composer manifests (composer.json) and the lock file. * - * @internal Meant to be used only inside the Flow package management code. + * @internal Only meant to be used only inside the Flow package management code. */ class ComposerUtility { diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index 0754588947..43dac75c79 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -20,7 +20,7 @@ * The idea around the Flow package key is obsolete since composer and will eventually be replaced. * Still major parts of Flow depend on the concept. * - * @internal Only meant to be used inside the Flow core until replaced by composer keys. + * @internal Only meant to be used only inside the Flow package management code. Should NOT leak into public APIs. */ final readonly class FlowPackageKey implements \JsonSerializable { diff --git a/Neos.Flow/Classes/Package/PackageManager.php b/Neos.Flow/Classes/Package/PackageManager.php index a9ea1d238d..799c22d59a 100644 --- a/Neos.Flow/Classes/Package/PackageManager.php +++ b/Neos.Flow/Classes/Package/PackageManager.php @@ -175,11 +175,11 @@ public function getFlowPackages(): array * Returns true if a package is available (the package's files exist in the packages directory) * or false if it's not. * - * @param string|FlowPackageKey $packageKey The key of the package to check + * @param string $packageKey The key of the package to check * @return boolean true if the package is available, otherwise false * @api */ - public function isPackageAvailable(string|FlowPackageKey $packageKey): bool + public function isPackageAvailable($packageKey): bool { return ($this->getCaseSensitivePackageKey($packageKey) !== false); } @@ -197,16 +197,15 @@ public function getPackagesBasePath(): string /** * Returns a PackageInterface object for the specified package. * - * @param string|FlowPackageKey $packageKey + * @param string $packageKey * @return PackageInterface The requested package object * @throws Exception\UnknownPackageException if the specified package is not known * @api */ - public function getPackage(string|FlowPackageKey $packageKey): PackageInterface + public function getPackage($packageKey): PackageInterface { if (!$this->isPackageAvailable($packageKey)) { - $packageKeyString = $packageKey instanceof FlowPackageKey ? $packageKey->value : $packageKey; - throw new Exception\UnknownPackageException('Package "' . $packageKeyString . '" is not available. Please check if the package exists and that the package key is correct (package keys are case sensitive).', 1166546734); + throw new Exception\UnknownPackageException('Package "' . $packageKey . '" is not available. Please check if the package exists and that the package key is correct (package keys are case sensitive).', 1166546734); } return $this->packages[$this->getCaseSensitivePackageKey($packageKey)]; @@ -315,12 +314,10 @@ protected function filterPackagesByType($packages, $packageType): array * @throws InvalidConfigurationException * @api */ - public function createPackage(string|FlowPackageKey $packageKey, array $manifest = [], $packagesPath = null): PackageInterface + public function createPackage($packageKey, array $manifest = [], $packagesPath = null): PackageInterface { - if (!$packageKey instanceof FlowPackageKey) { - $packageKey = FlowPackageKey::fromString($packageKey); - } - if ($this->isPackageAvailable($packageKey)) { + $packageKey = FlowPackageKey::fromString($packageKey); + if ($this->isPackageAvailable($packageKey->value)) { throw new Exception\PackageKeyAlreadyExistsException('The package key "' . $packageKey->value . '" already exists', 1220722873); } if (!isset($manifest['type'])) { @@ -783,15 +780,13 @@ protected function collectPackageManifestData(array $packageStates): array * Returns the correctly cased version of the given package key or false * if no such package is available. * - * @param string|FlowPackageKey $unknownCasedPackageKey The package key to convert + * @param string $unknownCasedPackageKey The package key to convert * @return string|false The upper camel cased package key or false if no such package exists * @api */ - public function getCaseSensitivePackageKey(string|FlowPackageKey $unknownCasedPackageKey): string|false + public function getCaseSensitivePackageKey($unknownCasedPackageKey) { - $lowerCasedPackageKey = strtolower( - $unknownCasedPackageKey instanceof FlowPackageKey ? $unknownCasedPackageKey->value : $unknownCasedPackageKey - ); + $lowerCasedPackageKey = strtolower($unknownCasedPackageKey); return $this->packageKeys[$lowerCasedPackageKey] ?? false; } @@ -824,7 +819,7 @@ public function getPackageKeyFromComposerName($composerName): string * * @param string $packageKey The package key to validate * @return boolean If the package key is valid, returns true otherwise false - * @deprecated with Flow 9.0 {@see FlowPackageKey::isPackageKeyValid()} + * @api */ public function isPackageKeyValid($packageKey): bool { From 73f4ff8081bc343506561d704d5fb87c37b8f554 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 29 Mar 2024 11:51:22 +0100 Subject: [PATCH 11/13] TASK: Replace usages of `PATTERN_MATCH_PACKAGEKEY` --- Neos.Flow/Classes/Package/PackageInterface.php | 2 +- .../Core/Parser/Interceptor/ResourceInterceptor.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Neos.Flow/Classes/Package/PackageInterface.php b/Neos.Flow/Classes/Package/PackageInterface.php index 09c3550812..b8ce3e2efd 100644 --- a/Neos.Flow/Classes/Package/PackageInterface.php +++ b/Neos.Flow/Classes/Package/PackageInterface.php @@ -18,7 +18,7 @@ */ interface PackageInterface { - /** @deprecated please use the FlowPackageKey instead */ + /** @deprecated with Flow 9, please use {@see PackageManager::isPackageKeyValid()} instead. */ const PATTERN_MATCH_PACKAGEKEY = FlowPackageKey::PATTERN; const DEFAULT_COMPOSER_TYPE = 'neos-package'; diff --git a/Neos.FluidAdaptor/Classes/Core/Parser/Interceptor/ResourceInterceptor.php b/Neos.FluidAdaptor/Classes/Core/Parser/Interceptor/ResourceInterceptor.php index 735d43351f..286870a6ac 100644 --- a/Neos.FluidAdaptor/Classes/Core/Parser/Interceptor/ResourceInterceptor.php +++ b/Neos.FluidAdaptor/Classes/Core/Parser/Interceptor/ResourceInterceptor.php @@ -11,7 +11,7 @@ * source code. */ -use Neos\Flow\Package\Package; +use Neos\Flow\Package\FlowPackageKey; use Neos\FluidAdaptor\Core\Parser\SyntaxTree\ResourceUriNode; use TYPO3Fluid\Fluid\Core\Parser\InterceptorInterface; use TYPO3Fluid\Fluid\Core\Parser\ParsingState; @@ -53,7 +53,7 @@ class ResourceInterceptor implements InterceptorInterface * Is the text at hand a resource URI and what are path/package? * * @var string - * @see \Neos\Flow\Package\Package::PATTERN_MATCH_PACKAGEKEY + * @see \Neos\Flow\Package\FlowPackageKey::PATTERN */ const PATTERN_MATCH_RESOURCE_URI = '!(?:../)*(?:(?P[A-Za-z0-9]+\.(?:[A-Za-z0-9][\.a-z0-9]*)+)/Resources/)?Public/(?P[^"]+)!'; @@ -74,7 +74,7 @@ class ResourceInterceptor implements InterceptorInterface */ public function setDefaultPackageKey($defaultPackageKey) { - if (!preg_match(Package::PATTERN_MATCH_PACKAGEKEY, $defaultPackageKey)) { + if (!FlowPackageKey::isPackageKeyValid($defaultPackageKey)) { throw new \InvalidArgumentException('The given argument was not a valid package key.', 1277287099); } $this->defaultPackageKey = $defaultPackageKey; @@ -109,7 +109,7 @@ public function process(NodeInterface $node, $interceptorPosition, ParsingState if ($this->defaultPackageKey !== null) { $arguments['package'] = new TextNode($this->defaultPackageKey); } - if (isset($matches['Package']) && preg_match(Package::PATTERN_MATCH_PACKAGEKEY, $matches['Package'])) { + if (isset($matches['Package']) && FlowPackageKey::isPackageKeyValid($matches['Package'])) { $arguments['package'] = new TextNode($matches['Package']); } From b7cadb58635acdc8de12945e4a86c28fa98c77a7 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 29 Mar 2024 12:59:14 +0100 Subject: [PATCH 12/13] TASK: Add FlowPackageKey tests and adjust namings --- .../Classes/Composer/ComposerUtility.php | 2 +- Neos.Flow/Classes/Package/FlowPackageKey.php | 22 +-- Neos.Flow/Classes/Package/PackageManager.php | 2 +- .../Tests/Unit/Package/FlowPackageKeyTest.php | 175 ++++++++++++++++++ .../Tests/Unit/Package/PackageManagerTest.php | 2 +- 5 files changed, 188 insertions(+), 15 deletions(-) create mode 100644 Neos.Flow/Tests/Unit/Package/FlowPackageKeyTest.php diff --git a/Neos.Flow/Classes/Composer/ComposerUtility.php b/Neos.Flow/Classes/Composer/ComposerUtility.php index b729a46693..fa18783685 100644 --- a/Neos.Flow/Classes/Composer/ComposerUtility.php +++ b/Neos.Flow/Classes/Composer/ComposerUtility.php @@ -144,7 +144,7 @@ public static function writeComposerManifest(string $manifestPath, FlowPackageKe $manifest = array_merge($manifest, $composerManifestData); } if (!isset($manifest['name']) || empty($manifest['name'])) { - $manifest['name'] = $packageKey->guessComposerPackageName(); + $manifest['name'] = $packageKey->deriveComposerPackageName(); } if (!isset($manifest['require']) || empty($manifest['require'])) { diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index 43dac75c79..017b99964e 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -14,8 +14,8 @@ * But before the adaption Flow already established the package keys like "Vendor.Foo.Bar", * which is represented and validated by this value object. * - * The Flow package keys are currently inferred from the composer manifest {@see FlowPackageKey::getPackageKeyFromManifest()}, - * and can also be tried to be reverse calculated: {@see FlowPackageKey::guessComposerPackageName()} + * The Flow package keys are currently inferred from the composer manifest {@see FlowPackageKey::deriveFromManifestOrPath()}, + * and can also be tried to be reverse calculated: {@see FlowPackageKey::deriveComposerPackageName()} * * The idea around the Flow package key is obsolete since composer and will eventually be replaced. * Still major parts of Flow depend on the concept. @@ -63,7 +63,7 @@ public static function isPackageKeyValid(string $string): bool * * Else the composer name will be used with the slash replaced by a dot */ - public static function getPackageKeyFromManifest(array $manifest, string $packagePath): self + public static function deriveFromManifestOrPath(array $manifest, string $packagePath): self { $definedFlowPackageKey = $manifest['extra']['neos']['package-key'] ?? null; @@ -73,17 +73,12 @@ public static function getPackageKeyFromManifest(array $manifest, string $packag $composerName = $manifest['name']; $autoloadNamespace = null; - $type = null; + $type = $manifest['type'] ?? null; if (isset($manifest['autoload']['psr-0']) && is_array($manifest['autoload']['psr-0'])) { $namespaces = array_keys($manifest['autoload']['psr-0']); $autoloadNamespace = reset($namespaces); } - - if (isset($manifest['type'])) { - $type = $manifest['type']; - } - - return self::derivePackageKey($composerName, $type, $packagePath, $autoloadNamespace); + return self::derivePackageKeyInternal($composerName, $type, $packagePath, $autoloadNamespace); } /** @@ -94,7 +89,7 @@ public static function getPackageKeyFromManifest(array $manifest, string $packag * - first found autoload namespace * - composer name */ - private static function derivePackageKey(string $composerName, ?string $packageType, string $packagePath, ?string $autoloadNamespace): self + private static function derivePackageKeyInternal(string $composerName, ?string $packageType, string $packagePath, ?string $autoloadNamespace): self { $packageKey = ''; @@ -121,8 +116,11 @@ private static function derivePackageKey(string $composerName, ?string $packageT /** * Determines the composer package name ("vendor/foo-bar") from the Flow package key ("Vendor.Foo.Bar") + * + * TODO: This is NOT necessary the reverse calculation when the package key was inferred via {@see self::deriveFromManifestOrPath()} + * For example vendor/foo-bar will become vendor.foobar which in turn will be converted via this method to vendor/foobar */ - public function guessComposerPackageName(): string + public function deriveComposerPackageName(): string { $nameParts = explode('.', $this->value); $vendor = array_shift($nameParts); diff --git a/Neos.Flow/Classes/Package/PackageManager.php b/Neos.Flow/Classes/Package/PackageManager.php index 799c22d59a..41d6f2ce7d 100644 --- a/Neos.Flow/Classes/Package/PackageManager.php +++ b/Neos.Flow/Classes/Package/PackageManager.php @@ -590,7 +590,7 @@ protected function scanAvailablePackages(): array continue; } - $packageKey = FlowPackageKey::getPackageKeyFromManifest($composerManifest, $packagePath); + $packageKey = FlowPackageKey::deriveFromManifestOrPath($composerManifest, $packagePath); $this->composerNameToPackageKeyMap[strtolower($composerManifest['name'])] = $packageKey->value; $packageConfiguration = $this->preparePackageStateConfiguration($packageKey, $packagePath, $composerManifest); diff --git a/Neos.Flow/Tests/Unit/Package/FlowPackageKeyTest.php b/Neos.Flow/Tests/Unit/Package/FlowPackageKeyTest.php new file mode 100644 index 0000000000..3d9262132e --- /dev/null +++ b/Neos.Flow/Tests/Unit/Package/FlowPackageKeyTest.php @@ -0,0 +1,175 @@ +value); + } + + /** + * @dataProvider invalidPackageKeys + * @test + */ + public function invalidPackageKeysAreRejected(string $packageKey) + { + $this->expectException(InvalidPackageKeyException::class); + FlowPackageKey::fromString($packageKey); + } + + public static function deriveFromManifestOrPathExamples(): iterable + { + yield 'for libraries the package key inferred from composer name' => [ + 'manifest' => json_decode(<<<'JSON' + { + "name": "my/package" + } + JSON, true, 512, JSON_THROW_ON_ERROR), + 'packagePath' => '', + 'expected' => 'my.package' + ]; + + yield 'for libraries the package key inferred from composer name (hyphens are removed)' => [ + 'manifest' => json_decode(<<<'JSON' + { + "name": "vendor/foo-bar" + } + JSON, true, 512, JSON_THROW_ON_ERROR), + 'packagePath' => '', + 'expected' => 'vendor.foobar' + ]; + + yield 'for type neos packages the case sensitive name from the folder is used' => [ + 'manifest' => json_decode(<<<'JSON' + { + "name": "my/flow-package", + "type": "neos-plugin" + } + JSON, true, 512, JSON_THROW_ON_ERROR), + 'packagePath' => '/app/Packages/Framework/My.Flow.Package', + 'expected' => 'My.Flow.Package' + ]; + + yield 'empty defined Flow package key' => [ + 'manifest' => json_decode(<<<'JSON' + { + "name": "my/package", + "extra": { + "neos": { + "package-key": "" + } + } + } + JSON, true, 512, JSON_THROW_ON_ERROR), + 'packagePath' => '', + 'expected' => 'my.package' + ]; + + + yield 'invalid defined Flow package key' => [ + 'manifest' => json_decode(<<<'JSON' + { + "name": "my/package", + "extra": { + "neos": { + "package-key": "Floh:" + } + } + } + JSON, true, 512, JSON_THROW_ON_ERROR), + 'packagePath' => '', + 'expected' => 'my.package' + ]; + + yield 'explicitly (differently) defined Flow package key' => [ + 'manifest' => json_decode(<<<'JSON' + { + "name": "my/package", + "extra": { + "neos": { + "package-key": "MyCustom.PackageKey" + } + } + } + JSON, true, 512, JSON_THROW_ON_ERROR), + 'packagePath' => '', + 'expected' => 'MyCustom.PackageKey' + ]; + + yield 'legacy psr-0 autoload path will be used as fallback' => [ + 'manifest' => json_decode(<<<'JSON' + { + "name": "acme/mypackage", + "autoload": { + "psr-0": { + "Acme\\MyPackage": "Classes/" + } + } + } + JSON, true, 512, JSON_THROW_ON_ERROR), + 'packagePath' => '', + 'expected' => 'Acme.MyPackage' + ]; + } + + /** + * @dataProvider deriveFromManifestOrPathExamples + * @test + */ + public function deriveFromManifestOrPath(array $manifest, string $packagePath, string $expected) + { + $actual = FlowPackageKey::deriveFromManifestOrPath($manifest, $packagePath); + self::assertSame($expected, $actual->value); + } + + /** + * @test + */ + public function deriveComposerPackageName() + { + self::assertSame( + 'neos/flow', + FlowPackageKey::fromString('Neos.Flow')->deriveComposerPackageName() + ); + + self::assertSame( + 'vendor/foo-bar', + FlowPackageKey::fromString('Vendor.Foo.Bar')->deriveComposerPackageName() + ); + } +} diff --git a/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php b/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php index 017d2ed6da..60caf74770 100644 --- a/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php +++ b/Neos.Flow/Tests/Unit/Package/PackageManagerTest.php @@ -262,7 +262,7 @@ public function packageStatesConfigurationContainsRelativePaths() $expectedPackageStatesConfiguration = []; foreach ($packageKeys as $packageKey) { - $composerName = $packageKey->guessComposerPackageName(); + $composerName = $packageKey->deriveComposerPackageName(); $expectedPackageStatesConfiguration[$composerName] = [ 'packagePath' => 'Application/' . $packageKey->value . '/', 'composerName' => $composerName, From 270c8bde70f3c020f0887deef6a50b054fe061f2 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 31 Mar 2024 17:22:17 +0200 Subject: [PATCH 13/13] TASK: Adjust `FlowPackageKey` documentation --- Neos.Flow/Classes/Package/FlowPackageKey.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Neos.Flow/Classes/Package/FlowPackageKey.php b/Neos.Flow/Classes/Package/FlowPackageKey.php index 017b99964e..5e5a72ec6e 100644 --- a/Neos.Flow/Classes/Package/FlowPackageKey.php +++ b/Neos.Flow/Classes/Package/FlowPackageKey.php @@ -22,7 +22,7 @@ * * @internal Only meant to be used only inside the Flow package management code. Should NOT leak into public APIs. */ -final readonly class FlowPackageKey implements \JsonSerializable +final readonly class FlowPackageKey { public const PATTERN = '/^[a-z0-9]+\.(?:[a-z0-9][\.a-z0-9]*)+$/i'; @@ -47,6 +47,7 @@ public static function fromString(string $value) * * @param string $string The package key to validate * @return boolean If the package key is valid, returns true otherwise false + * @internal please use {@see PackageManager::isPackageKeyValid()} */ public static function isPackageKeyValid(string $string): bool { @@ -117,7 +118,7 @@ private static function derivePackageKeyInternal(string $composerName, ?string $ /** * Determines the composer package name ("vendor/foo-bar") from the Flow package key ("Vendor.Foo.Bar") * - * TODO: This is NOT necessary the reverse calculation when the package key was inferred via {@see self::deriveFromManifestOrPath()} + * WARNING: This is NOT necessary the reverse calculation when the package key was inferred via {@see self::deriveFromManifestOrPath()} * For example vendor/foo-bar will become vendor.foobar which in turn will be converted via this method to vendor/foobar */ public function deriveComposerPackageName(): string @@ -126,9 +127,4 @@ public function deriveComposerPackageName(): string $vendor = array_shift($nameParts); return strtolower($vendor . '/' . implode('-', $nameParts)); } - - public function jsonSerialize(): string - { - return $this->value; - } }