Skip to content

Commit

Permalink
[BUGFIX] Avoid resolving invalid TYPO3 extensions in `ComposerPackage…
Browse files Browse the repository at this point in the history
…Manager`

The `ComposerPackageManager` has been introduced to streamline
the functional test instance creation process and provide all
selected extensions (system, custom and test fixture) within
the test instance, which original simply used relative path
names for classic mode instances or a simple extension key:

For `$coreExtensionsToLoad`:

* typo3/sysext/backend
* backend

For `$testExtensionsToLoad`:

* typo3conf/ext/my_ext_key
* my_ext_key

With `typo3/cms-composer-installers` version 4.0RC1 and
5 these paths could not be found anymore, because TYPO3
system extensions and extensions are no longer installed
into the classic paths in a composer mode instance and
left in the vendor folder, which is the case for usual
root project or extension instance.

Using the available composer information to determine the
source for extensions unrelated to the real installation
path, which can be configured with composer, was the way
to mitigate this issue and `ComposerPackageManger` has
been implemented to process these lookups while still
supporting test fixture extensions not loaded by the root
composer.json directly.

The implementation tried to provide backwards compatible
as much as possible along with fallback to use folder names
as extension keys by simply using `basename()` in some code
places and including not obvious side effects and lookup
issues. `basename()` was also used on valid composer package
names to resolve composer package name for that value as
extension key for loaded packages, which leads to fetch the
wrong composer package.

The standlone fluid `typo3fluid/fluid` composer package name
resolved and retrieved the TYPO3 system extension package
`typo3/cms-fluid` using `getPackageInfo()`, which lead to
issues in other places, for example the dependency ordering
and resolving class `PackageCollection`.

This change streamlines the `ComposerPackageManager` class
to mitigate building and using invalid values to lookup
extension composer package names and harden the registration
process of extension even further.

Guarding unit tests are added to cover this bugfix.

Resolves: #553
Releases: main, 8
  • Loading branch information
sbuerk committed Nov 28, 2024
1 parent 21213bd commit 9a1fada
Show file tree
Hide file tree
Showing 5 changed files with 425 additions and 22 deletions.
91 changes: 83 additions & 8 deletions Classes/Composer/ComposerPackageManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,22 @@
*/

use Composer\InstalledVersions;
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;

/**
* `typo3/testing-framework` internal composer package manager, used to gather source
* information of extensions already loaded by the root composer installation with
* the additional ability to register test fixture packages and extensions during
* runtime to create {@see FunctionalTestCase} test instances and provide symlinks
* of extensions into the classic mode test instance or retrieve files from a composer
* package or extension unrelated where they are placed on the filesystem.
*
* - {@see Testbase::setUpInstanceCoreLinks()}
* - {@see Testbase::linkTestExtensionsToInstance()}
* - {@see Testbase::linkFrameworkExtensionsToInstance()}
* - {@see Testbase::setUpLocalConfiguration()}
* - {@see Testbase::setUpPackageStates()}
*
* @internal This class is for testing-framework internal processing and not part of public testing API.
*/
final class ComposerPackageManager
Expand Down Expand Up @@ -65,25 +79,29 @@ public function __construct()
$this->build();
}

public function getPackageInfoWithFallback(string $name): ?PackageInfo
/**
* Get composer package information {@see PackageInfo} for `$nameOrExtensionKeyOrPath`.
*/
public function getPackageInfoWithFallback(string $nameOrExtensionKeyOrPath): ?PackageInfo
{
if ($packageInfo = $this->getPackageInfo($name)) {
if ($packageInfo = $this->getPackageInfo($nameOrExtensionKeyOrPath)) {
return $packageInfo;
}
if ($packageInfo = $this->getPackageFromPath($name)) {
if ($packageInfo = $this->getPackageFromPath($nameOrExtensionKeyOrPath)) {
return $packageInfo;
}
if ($packageInfo = $this->getPackageFromPathFallback($name)) {
if ($packageInfo = $this->getPackageFromPathFallback($nameOrExtensionKeyOrPath)) {
return $packageInfo;
}

return null;
}

/**
* Get {@see PackageInfo} for package name or extension key `$name`.
*/
public function getPackageInfo(string $name): ?PackageInfo
{
$name = $this->resolvePackageName($name);
return self::$packages[$name] ?? null;
return self::$packages[$this->resolvePackageName($name)] ?? null;
}

/**
Expand Down Expand Up @@ -403,9 +421,23 @@ private function getExtEmConf(string $path): ?array
return null;
}

/**
* Returns resolved composer package name when $name is a known extension key
* for a known package, otherwise return $name unchanged.
*
* Used to determine the package name to look up as composer package within {@see self::$packages}
*
* Supports also relative classic mode notation:
*
* - typo3/sysext/backend
* - typo3conf/ext/my_ext_key
*
* {@see self::prepareResolvePackageName()} for details for normalisation.
*/
private function resolvePackageName(string $name): string
{
return self::$extensionKeyToPackageNameMap[$this->normalizeExtensionKey(basename($name))] ?? $name;
$name = $this->prepareResolvePackageName($name);
return self::$extensionKeyToPackageNameMap[$name] ?? $name;
}

/**
Expand Down Expand Up @@ -640,4 +672,47 @@ private function getFirstPathElement(string $path): string
}
return explode('/', $path)[0] ?? '';
}

/**
* Extension can be specified with their composer name, extension key or with classic mode relative path
* prefixes (`typo3/sysext/<extensionkey>` or `typo3conf/ext/<extensionkey>`) for functional tests to
* configure which extension should be provided in the test instance.
*
* This method normalizes a handed over name by removing the specified extra information, so it can be
* used to resolve it either as direct package name or as extension name.
*
* Handed over value also removes known environment prefix paths, like the full path to the root (project rook),
* vendor folder or web folder using {@see self::removePrefixPaths()} which is safe, as this method is and most
* only be used for {@see self::resolvePackageName()} to find a composer package in {@see self::$packages}, after
* mapping extension-key to composer package name.
*
* Example for processed changes:
* -----------------------------_
*
* - typo3/sysext/backend => backend
* - typo3conf/ext/my_ext_key => my_ext_key
*
* Example not processed values:
* -----------------------------
*
* valid names
* - typo3/cms-core => typo3/cms-core
* - my-vendor/my-package-name => my-vendor/my-package-name
* - my-package-name-without-vendor => my-package-name-without-vendor
*/
private function prepareResolvePackageName($name): string
{
$name = trim($this->removePrefixPaths($name), '/');
$relativePrefixPaths = [
'typo3/sysext/',
'typo3conf/ext/',
];
foreach ($relativePrefixPaths as $relativePrefixPath) {
if (!str_starts_with($name, $relativePrefixPath)) {
continue;
}
$name = substr($name, mb_strlen($relativePrefixPath));
}
return $name;
}
}
69 changes: 55 additions & 14 deletions Classes/Core/Functional/FunctionalTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,23 @@ abstract class FunctionalTestCase extends BaseTestCase implements ContainerInter
*
* A default list of core extensions is always loaded.
*
* System extension can be provided by their extension key or composer package name,
* and also as classic mode relative path
*
* ```
* protected array $coreExensionToLoad = [
* // As composer package name
* 'typo3/cms-core',
* // As extension-key
* 'core',
* // As relative classic mode system installation path
* 'typo3/sysext/core',
* ];
* ```
*
* Note that system extensions must be available, which means either added as require or
* require-dev to the root composer.json or required and installed by a required package.
*
* @see FunctionalTestCaseUtility $defaultActivatedCoreExtensions
*
* @var non-empty-string[]
Expand All @@ -121,16 +138,32 @@ abstract class FunctionalTestCase extends BaseTestCase implements ContainerInter
* Array of test/fixture extensions paths that should be loaded for a test.
*
* This property will stay empty in this abstract, so it is possible
* to just overwrite it in extending classes. Extensions noted here will
* be loaded for every test of a test case, and it is not possible to change
* the list of loaded extensions between single tests of a test case.
* to just overwrite it in extending classes.
*
* IMPORTANT: Extension list is concrete and used to create the test instance on first
* test execution and is **NOT** changeable between single test permutations.
*
* Given path is expected to be relative to your document root, example:
*
* array(
* 'typo3conf/ext/some_extension/Tests/Functional/Fixtures/Extensions/test_extension',
* ```
* protected array $testExtensionToLoad = [
*
* // Virtual relative classic mode installation path
* 'typo3conf/ext/base_extension',
* );
*
* // Virtual relative classic mode installation path subfolder test fixture
* 'typo3conf/ext/some_extension/Tests/Functional/Fixtures/Extensions/test_extension',
*
* // Relative to current test case (recommended for test fixture extension)
* __DIR__ . '/../Fixtures/Extensions/another_test_extension',
*
* // composer package name when available as `require` or `require-dev` in root composer.json
* 'vendor/some-extension',
*
* // extension key when available as package loaded as `require` or `require-dev` in root composer.json
* 'my_extension_key',
* ];
* ```
*
* Extensions in this array are linked to the test instance, loaded
* and their ext_tables.sql will be applied.
Expand All @@ -147,18 +180,22 @@ abstract class FunctionalTestCase extends BaseTestCase implements ContainerInter
* be linked for every test of a test case, and it is not possible to change
* the list of folders between single tests of a test case.
*
* array(
* ```
* protected array $pathsToLinkInTestInstance = [
* 'link-source' => 'link-destination'
* );
* ];
* ```
*
* Given paths are expected to be relative to the test instance root.
* The array keys are the source paths and the array values are the destination
* paths, example:
*
* [
* ```
* protected array $pathsToLinkInTestInstance = [
* 'typo3/sysext/impext/Tests/Functional/Fixtures/Folders/fileadmin/user_upload' =>
* 'fileadmin/user_upload',
* ]
* ];
* ```
*
* To be able to link from my_own_ext the extension path needs also to be registered in
* property $testExtensionsToLoad
Expand All @@ -172,12 +209,14 @@ abstract class FunctionalTestCase extends BaseTestCase implements ContainerInter
* paths are really duplicated and provided in the instance - instead of
* using symbolic links. Examples:
*
* [
* ```
* protected array $pathsToProvideInTestInstance = [
* // Copy an entire directory recursive to fileadmin
* 'typo3/sysext/lowlevel/Tests/Functional/Fixtures/testImages/' => 'fileadmin/',
* // Copy a single file into some deep destination directory
* 'typo3/sysext/lowlevel/Tests/Functional/Fixtures/testImage/someImage.jpg' => 'fileadmin/_processed_/0/a/someImage.jpg',
* ]
* ];
* ```
*
* @var array<string, non-empty-string>
*/
Expand Down Expand Up @@ -208,9 +247,11 @@ abstract class FunctionalTestCase extends BaseTestCase implements ContainerInter
* To create additional folders add the paths to this array. Given paths are expected to be
* relative to the test instance root and have to begin with a slash. Example:
*
* [
* ```
* protected array $additionalFoldersToCreate = [
* 'fileadmin/user_upload'
* ]
* ];
* ```
*
* @var non-empty-string[]
*/
Expand Down
Loading

0 comments on commit 9a1fada

Please sign in to comment.