Skip to content

Commit

Permalink
[BUGFIX] Respect composer mode only extension in FunctionalTestCase
Browse files Browse the repository at this point in the history
The `typo3/testing-framework` creates functional test instances
as "classic" mode instances, writing a `PackageStates.php` file
composed using composer information, provided and handled by the
internal `ComposerPackageManager` class. Extensions in the state
file are not sorted based on their dependencies and suggestions.

To mitigate this and having a correctly sorted extension state
file, the `PackageCollection` service has been introduced with
the goal to resort the extension state file after the initial
write to provide the correct sorted extension state.

The extension sorting within the state file is important, as the
TYPO3 Core uses this sorting to loop over extensions to collect
information from the extensions, for example TCA, TCAOverrides,
ext_localconf.php and other things. Package sorting is here very
important to allow extensions to reconfigure or change the stuff
from other extensions, which is guaranteed in normal "composer"
and "classic" mode instances.

For "classic" mode instances, only the `ext_emconf.php` file is
taken into account and "composer.json" as the source of thruth
for "composer" mode instances since TYPO3 v12, which made the
`ext_emconf.php` file obsolete for extensions only installed
with composer.

Many agencies removed the optional `ext_emconf.php` file for
project local path extensions like a sitepackage to remove
the maintenance burden, which is a valid case.

Sadly, the `PackageCollection` adopted from the TYPO3 Core
`PackageManager` did not reflected this case and failed for
extensions to properly sort only on extension dependencies
and keys due the mix of extension key and composer package
name handling. Extension depending on another extension
failed to be sorted correctly with following exception:

    UnexpectedValueException: The package "extension_key"
    depends on "composer/package-key" which is not present
    in the system.

This change modifies the `PackageCollection` implementation
to take only TYPO3 extension and system extension into account
for dependency resolving and sorting, using `ext_emconf.php`
depends/suggest information as first source and falling back
to `composer` require and suggestion information.

Resolves: #541
Releases: main, 8
  • Loading branch information
DanielSiepmann authored and sbuerk committed Nov 28, 2024
1 parent 0fa866c commit 1889534
Show file tree
Hide file tree
Showing 17 changed files with 529 additions and 28 deletions.
2 changes: 2 additions & 0 deletions Build/phpstan/phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ parameters:
- ../../Classes/Core/Acceptance/*
# Text fixtures extensions uses $_EXTKEY phpstan would be report as "might not defined"
- ../../Tests/Unit/*/Fixtures/Extensions/*/ext_emconf.php
- ../../Tests/Unit/*/Fixtures/Packages/*/ext_emconf.php
- ../../Tests/Unit/Fixtures/Packages/*/ext_emconf.php
16 changes: 13 additions & 3 deletions Classes/Composer/ComposerPackageManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ final class ComposerPackageManager

public function __construct()
{
// @todo Remove this from the constructor.
$this->build();
}

Expand Down Expand Up @@ -545,11 +546,20 @@ private function determineExtensionKey(
?array $info = null,
?array $extEmConf = null
): string {
$isExtension = in_array($info['type'] ?? '', ['typo3-cms-framework', 'typo3-cms-extension'], true)
|| ($extEmConf !== null);
if (!$isExtension) {
$isComposerExtensionType = ($info !== null && array_key_exists('type', $info) && is_string($info['type']) && in_array($info['type'], ['typo3-cms-framework', 'typo3-cms-extension'], true));
$hasExtEmConf = $extEmConf !== null;
if (!($isComposerExtensionType || $hasExtEmConf)) {
return '';
}
$hasComposerExtensionKey = (
is_array($info)
&& isset($info['extra']['typo3/cms']['extension-key'])
&& is_string($info['extra']['typo3/cms']['extension-key'])
&& $info['extra']['typo3/cms']['extension-key'] !== ''
);
if ($hasComposerExtensionKey) {
return $info['extra']['typo3/cms']['extension-key'];
}
$baseName = basename($packagePath);
if (($info['type'] ?? '') === 'typo3-csm-framework'
&& str_starts_with($baseName, 'cms-')
Expand Down
128 changes: 103 additions & 25 deletions Classes/Core/PackageCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,38 @@
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\PathUtility;
use TYPO3\TestingFramework\Composer\ComposerPackageManager;
use TYPO3\TestingFramework\Composer\PackageInfo;
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;

/**
* Collection for extension packages to resolve their dependencies in a test-base.
* Most of the code has been duplicated and adjusted from `\TYPO3\CMS\Core\Package\PackageManager`.
* Composer package collection to resolve extension dependencies for classic-mode based test instances.
*
* This class resolves extension dependencies for composer packages to sort classic-mode PackageStates,
* which only takes TYPO3 extensions into account with a fallback to read composer information when the
* `ext_emconf.php` file is missing.
*
* Most of the code has been duplicated and adjusted from {@see PackageManager}.
*
* Background:
* ===========
*
* TYPO3 has the two installation mode "composer" and "classic". For the "composer" mode the package dependency handling
* is mainly done by composer and dependency detection and sorting is purely based on composer.json information. "Classic"
* mode uses only "ext_emconf.php" information to do the same job, not mixing it with the composer.json information when
* available.
*
* Since TYPO3 v12 extensions installed in "composer" mode are not required to provide a "ext_emconf.php" anymore, which
* makes them only installable within a "composer" mode installation. Agencies used to drop that file from local path
* extensions in "composer" mode projects, because it is a not needed requirement for them and avoids maintenance of it.
*
* typo3/testing-framework builds "classic" mode functional test instances while used within composer installations only,
* and introduced an extension sorting with this class to ensure to have a deterministic extension sorting like a real
* "classic" mode installation would provide in case extensions are not manually provided in the correct order within
* {@see FunctionalTestCase::$testExtensionToLoad} property.
*
* {@see PackageCollection} is based on the TYPO3 core {@see PackageManager} to provide a sorting for functional test
* instances, falling back to use composer.json information in case no "ext_emconf.php" are given limiting it only to
* TYPO3 compatible extensions (typo3-cms-framework and typo3-cms-extension composer package types).
*
* @phpstan-type PackageKey non-empty-string
* @phpstan-type PackageName non-empty-string
Expand All @@ -54,6 +82,10 @@ public static function fromPackageStates(ComposerPackageManager $composerPackage
{
$packages = [];
foreach ($packageStates as $packageKey => $packageStateConfiguration) {
// @todo Verify retrieving package information and throwing early exception after extension without
// composer.json support has been dropped, even for simplified test fixture extensions. Think
// about triggering deprecation for this case first, which may also breaking from a testing
// perspective.
$packagePath = PathUtility::sanitizeTrailingSeparator(
rtrim($basePath, '/') . '/' . $packageStateConfiguration['packagePath']
);
Expand Down Expand Up @@ -162,8 +194,11 @@ protected function convertConfigurationForGraph(array $allPackageConstraints, ar
];
if (isset($allPackageConstraints[$packageKey]['dependencies'])) {
foreach ($allPackageConstraints[$packageKey]['dependencies'] as $dependentPackageKey) {
if (!in_array($dependentPackageKey, $packageKeys, true)) {
if ($this->isComposerDependency($dependentPackageKey)) {
$extensionKey = $this->getPackageExtensionKey($dependentPackageKey);
if (!in_array($dependentPackageKey, $packageKeys, true)
&& !in_array($extensionKey, $packageKeys, true)
) {
if (!$this->isTypo3SystemOrCustomExtension($dependentPackageKey)) {
// The given package has a dependency to a Composer package that has no relation to TYPO3
// We can ignore those, when calculating the extension order
continue;
Expand All @@ -174,21 +209,30 @@ protected function convertConfigurationForGraph(array $allPackageConstraints, ar
1519931815
);
}
$dependencies[$packageKey]['after'][] = $dependentPackageKey;
$dependencies[$packageKey]['after'][] = $extensionKey;
}
}
if (isset($allPackageConstraints[$packageKey]['suggestions'])) {
foreach ($allPackageConstraints[$packageKey]['suggestions'] as $suggestedPackageKey) {
$extensionKey = $this->getPackageExtensionKey($suggestedPackageKey);
// skip suggestions on not existing packages
if (in_array($suggestedPackageKey, $packageKeys, true)) {
// Suggestions actually have never been meant to influence loading order.
// We misuse this currently, as there is no other way to influence the loading order
// for not-required packages (soft-dependency).
// When considering suggestions for the loading order, we might create a cyclic dependency
// if the suggested package already has a real dependency on this package, so the suggestion
// has do be dropped in this case and must *not* be taken into account for loading order evaluation.
$dependencies[$packageKey]['after-resilient'][] = $suggestedPackageKey;
if (!in_array($suggestedPackageKey, $packageKeys, true)
&& !in_array($extensionKey, $packageKeys, true)
) {
continue;
}
if (!$this->isTypo3SystemOrCustomExtension($extensionKey ?: $suggestedPackageKey)) {
// Ignore non TYPO3 extension packages for suggestion determination/ordering.
continue;
}

// Suggestions actually have never been meant to influence loading order.
// We misuse this currently, as there is no other way to influence the loading order
// for not-required packages (soft-dependency).
// When considering suggestions for the loading order, we might create a cyclic dependency
// if the suggested package already has a real dependency on this package, so the suggestion
// has do be dropped in this case and must *not* be taken into account for loading order evaluation.
$dependencies[$packageKey]['after-resilient'][] = $extensionKey;
}
}
}
Expand Down Expand Up @@ -255,25 +299,28 @@ protected function getDependencyArrayForPackage(PackageInterface $package, array
foreach ($dependentPackageConstraints as $constraint) {
if ($constraint instanceof PackageConstraint) {
$dependentPackageKey = $constraint->getValue();
if (!in_array($dependentPackageKey, $dependentPackageKeys, true) && !in_array($dependentPackageKey, $trace, true)) {
$dependentPackageKeys[] = $dependentPackageKey;
$extensionKey = $this->getPackageExtensionKey($dependentPackageKey) ?: $dependentPackageKey;
if (!in_array($extensionKey, $dependentPackageKeys, true)) {
$dependentPackageKeys[] = $extensionKey;
}
if (!isset($this->packages[$dependentPackageKey])) {
if ($this->isComposerDependency($dependentPackageKey)) {

if (!isset($this->packages[$extensionKey])) {
if (!$this->isTypo3SystemOrCustomExtension($extensionKey)) {
// The given package has a dependency to a Composer package that has no relation to TYPO3
// We can ignore those, when calculating the extension order
continue;
}

throw new Exception(
sprintf(
'Package "%s" depends on package "%s" which does not exist.',
$package->getPackageKey(),
$dependentPackageKey
$extensionKey
),
1695119749
);
}
$this->getDependencyArrayForPackage($this->packages[$dependentPackageKey], $dependentPackageKeys, $trace);
$this->getDependencyArrayForPackage($this->packages[$extensionKey], $dependentPackageKeys, $trace);
}
}
return array_reverse($dependentPackageKeys);
Expand All @@ -292,9 +339,17 @@ protected function getSuggestionArrayForPackage(PackageInterface $package): arra
foreach ($suggestedPackageConstraints as $constraint) {
if ($constraint instanceof PackageConstraint) {
$suggestedPackageKey = $constraint->getValue();
if (isset($this->packages[$suggestedPackageKey])) {
$suggestedPackageKeys[] = $suggestedPackageKey;
$extensionKey = $this->getPackageExtensionKey($suggestedPackageKey) ?: $suggestedPackageKey;
if (!$this->isTypo3SystemOrCustomExtension($suggestedPackageKey)) {
// Suggested packages which are not installed or not a TYPO3 extension can be skipped for
// sorting when not available.
continue;
}
if (!isset($this->packages[$extensionKey])) {
// Suggested extension is not available in test system installation (not symlinked), ignore it.
continue;
}
$suggestedPackageKeys[] = $extensionKey;
}
}
return array_reverse($suggestedPackageKeys);
Expand All @@ -308,15 +363,38 @@ protected function findFrameworkKeys(): array
$frameworkKeys = [];
foreach ($this->packages as $package) {
if ($package->getPackageMetaData()->isFrameworkType()) {
$frameworkKeys[] = $package->getPackageKey();
$frameworkKeys[] = $this->getPackageExtensionKey($package->getPackageKey()) ?: $package->getPackageKey();
}
}
return $frameworkKeys;
}

protected function isComposerDependency(string $packageKey): bool
/**
* Determines if given composer package key is either a `typo3-cms-framework` or `typo3-cms-extension` package.
*/
protected function isTypo3SystemOrCustomExtension(string $packageKey): bool
{
$packageInfo = $this->getPackageInfo($packageKey);
if ($packageInfo === null) {
return false;
}
return $packageInfo->isSystemExtension() || $packageInfo->isExtension();
}

/**
* Returns package extension key. Returns empty string if not available.
*/
protected function getPackageExtensionKey(string $packageKey): string
{
$packageInfo = $this->getPackageInfo($packageKey);
if ($packageInfo === null) {
return '';
}
return $packageInfo->getExtensionKey();
}

protected function getPackageInfo(string $packageKey): ?PackageInfo
{
$packageInfo = $this->composerPackageManager->getPackageInfo($packageKey);
return !(($packageInfo?->isSystemExtension() ?? false) || ($packageInfo?->isExtension()));
return $this->composerPackageManager->getPackageInfo($packageKey);
}
}
63 changes: 63 additions & 0 deletions Tests/Unit/Composer/ComposerPackageManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -295,4 +295,67 @@ public function extensionWithJsonCanBeResolvedByRelativeLegacyPath(): void
self::assertNotNull($packageInfo->getInfo());
self::assertNotNull($packageInfo->getExtEmConf());
}

public static function packagesWithoutExtEmConfFileDataProvider(): \Generator
{
yield 'package0 => package0' => [
'path' => __DIR__ . '/../Fixtures/Packages/package0',
'expectedExtensionKey' => 'package0',
'expectedPackageName' => 'typo3/testing-framework-package-0',
];
yield 'package0 => package1' => [
'path' => __DIR__ . '/../Fixtures/Packages/package1',
'expectedExtensionKey' => 'package1',
'expectedPackageName' => 'typo3/testing-framework-package-1',
];
yield 'package0 => package2' => [
'path' => __DIR__ . '/../Fixtures/Packages/package2',
'expectedExtensionKey' => 'package2',
'expectedPackageName' => 'typo3/testing-framework-package-2',
];
yield 'package-identifier => some_test_extension' => [
'path' => __DIR__ . '/../Fixtures/Packages/package-identifier',
'expectedExtensionKey' => 'some_test_extension',
'expectedPackageName' => 'typo3/testing-framework-package-identifier',
];
}

#[DataProvider('packagesWithoutExtEmConfFileDataProvider')]
#[Test]
public function getPackageInfoWithFallbackReturnsExtensionInfoWithCorrectExtensionKeyWhenNotHavingAnExtEmConfFile(
string $path,
string $expectedExtensionKey,
string $expectedPackageName,
): void {
$packageInfo = (new ComposerPackageManager())->getPackageInfoWithFallback($path);
self::assertInstanceOf(PackageInfo::class, $packageInfo, 'PackageInfo retrieved for ' . $path);
self::assertNull($packageInfo->getExtEmConf(), 'Package provides ext_emconf.php');
self::assertNotNull($packageInfo->getInfo(), 'Package has no composer info (composer.json)');
self::assertNotEmpty($packageInfo->getInfo(), 'Package composer info is empty');
self::assertTrue($packageInfo->isExtension(), 'Package is not a extension');
self::assertFalse($packageInfo->isSystemExtension(), 'Package is a system extension');
self::assertTrue($packageInfo->isComposerPackage(), 'Package is not a composer package');
self::assertFalse($packageInfo->isMonoRepository(), 'Package is mono repository');
self::assertSame($expectedPackageName, $packageInfo->getName());
self::assertSame($expectedExtensionKey, $packageInfo->getExtensionKey());
}

#[Test]
public function getPackageInfoWithFallbackReturnsExtensionInfoWithCorrectExtensionKeyAndHavingAnExtEmConfFile(): void
{
$path = __DIR__ . '/../Fixtures/Packages/package-with-extemconf';
$expectedExtensionKey = 'extension_with_extemconf';
$expectedPackageName = 'typo3/testing-framework-package-with-extemconf';
$packageInfo = (new ComposerPackageManager())->getPackageInfoWithFallback($path);
self::assertInstanceOf(PackageInfo::class, $packageInfo, 'PackageInfo retrieved for ' . $path);
self::assertNotNull($packageInfo->getExtEmConf(), 'Package has ext_emconf.php file');
self::assertNotNull($packageInfo->getInfo(), 'Package has composer info');
self::assertNotEmpty($packageInfo->getInfo(), 'Package composer info is not empty');
self::assertTrue($packageInfo->isExtension(), 'Package is a extension');
self::assertFalse($packageInfo->isSystemExtension(), 'Package is not a system extension');
self::assertTrue($packageInfo->isComposerPackage(), 'Package is a composer package');
self::assertFalse($packageInfo->isMonoRepository(), 'Package is not mono repository root');
self::assertSame($expectedPackageName, $packageInfo->getName());
self::assertSame($expectedExtensionKey, $packageInfo->getExtensionKey());
}
}
Loading

0 comments on commit 1889534

Please sign in to comment.