Skip to content

Commit c3d94c4

Browse files
DanielSiepmannsbuerk
authored andcommitted
[BUGFIX] Respect composer mode only extension in FunctionalTestCase
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
1 parent a6aa622 commit c3d94c4

File tree

17 files changed

+529
-28
lines changed

17 files changed

+529
-28
lines changed

Build/phpstan/phpstan.neon

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,5 @@ parameters:
2020
- ../../Classes/Core/Acceptance/*
2121
# Text fixtures extensions uses $_EXTKEY phpstan would be report as "might not defined"
2222
- ../../Tests/Unit/*/Fixtures/Extensions/*/ext_emconf.php
23+
- ../../Tests/Unit/*/Fixtures/Packages/*/ext_emconf.php
24+
- ../../Tests/Unit/Fixtures/Packages/*/ext_emconf.php

Classes/Composer/ComposerPackageManager.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ final class ComposerPackageManager
6161

6262
public function __construct()
6363
{
64+
// @todo Remove this from the constructor.
6465
$this->build();
6566
}
6667

@@ -545,11 +546,20 @@ private function determineExtensionKey(
545546
?array $info = null,
546547
?array $extEmConf = null
547548
): string {
548-
$isExtension = in_array($info['type'] ?? '', ['typo3-cms-framework', 'typo3-cms-extension'], true)
549-
|| ($extEmConf !== null);
550-
if (!$isExtension) {
549+
$isComposerExtensionType = ($info !== null && array_key_exists('type', $info) && is_string($info['type']) && in_array($info['type'], ['typo3-cms-framework', 'typo3-cms-extension'], true));
550+
$hasExtEmConf = $extEmConf !== null;
551+
if (!($isComposerExtensionType || $hasExtEmConf)) {
551552
return '';
552553
}
554+
$hasComposerExtensionKey = (
555+
is_array($info)
556+
&& isset($info['extra']['typo3/cms']['extension-key'])
557+
&& is_string($info['extra']['typo3/cms']['extension-key'])
558+
&& $info['extra']['typo3/cms']['extension-key'] !== ''
559+
);
560+
if ($hasComposerExtensionKey) {
561+
return $info['extra']['typo3/cms']['extension-key'];
562+
}
553563
$baseName = basename($packagePath);
554564
if (($info['type'] ?? '') === 'typo3-csm-framework'
555565
&& str_starts_with($baseName, 'cms-')

Classes/Core/PackageCollection.php

Lines changed: 103 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,38 @@
2626
use TYPO3\CMS\Core\Utility\GeneralUtility;
2727
use TYPO3\CMS\Core\Utility\PathUtility;
2828
use TYPO3\TestingFramework\Composer\ComposerPackageManager;
29+
use TYPO3\TestingFramework\Composer\PackageInfo;
30+
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
2931

3032
/**
31-
* Collection for extension packages to resolve their dependencies in a test-base.
32-
* Most of the code has been duplicated and adjusted from `\TYPO3\CMS\Core\Package\PackageManager`.
33+
* Composer package collection to resolve extension dependencies for classic-mode based test instances.
34+
*
35+
* This class resolves extension dependencies for composer packages to sort classic-mode PackageStates,
36+
* which only takes TYPO3 extensions into account with a fallback to read composer information when the
37+
* `ext_emconf.php` file is missing.
38+
*
39+
* Most of the code has been duplicated and adjusted from {@see PackageManager}.
40+
*
41+
* Background:
42+
* ===========
43+
*
44+
* TYPO3 has the two installation mode "composer" and "classic". For the "composer" mode the package dependency handling
45+
* is mainly done by composer and dependency detection and sorting is purely based on composer.json information. "Classic"
46+
* mode uses only "ext_emconf.php" information to do the same job, not mixing it with the composer.json information when
47+
* available.
48+
*
49+
* Since TYPO3 v12 extensions installed in "composer" mode are not required to provide a "ext_emconf.php" anymore, which
50+
* makes them only installable within a "composer" mode installation. Agencies used to drop that file from local path
51+
* extensions in "composer" mode projects, because it is a not needed requirement for them and avoids maintenance of it.
52+
*
53+
* typo3/testing-framework builds "classic" mode functional test instances while used within composer installations only,
54+
* and introduced an extension sorting with this class to ensure to have a deterministic extension sorting like a real
55+
* "classic" mode installation would provide in case extensions are not manually provided in the correct order within
56+
* {@see FunctionalTestCase::$testExtensionToLoad} property.
57+
*
58+
* {@see PackageCollection} is based on the TYPO3 core {@see PackageManager} to provide a sorting for functional test
59+
* instances, falling back to use composer.json information in case no "ext_emconf.php" are given limiting it only to
60+
* TYPO3 compatible extensions (typo3-cms-framework and typo3-cms-extension composer package types).
3361
*
3462
* @phpstan-type PackageKey non-empty-string
3563
* @phpstan-type PackageName non-empty-string
@@ -54,6 +82,10 @@ public static function fromPackageStates(ComposerPackageManager $composerPackage
5482
{
5583
$packages = [];
5684
foreach ($packageStates as $packageKey => $packageStateConfiguration) {
85+
// @todo Verify retrieving package information and throwing early exception after extension without
86+
// composer.json support has been dropped, even for simplified test fixture extensions. Think
87+
// about triggering deprecation for this case first, which may also breaking from a testing
88+
// perspective.
5789
$packagePath = PathUtility::sanitizeTrailingSeparator(
5890
rtrim($basePath, '/') . '/' . $packageStateConfiguration['packagePath']
5991
);
@@ -157,8 +189,11 @@ protected function convertConfigurationForGraph(array $allPackageConstraints, ar
157189
];
158190
if (isset($allPackageConstraints[$packageKey]['dependencies'])) {
159191
foreach ($allPackageConstraints[$packageKey]['dependencies'] as $dependentPackageKey) {
160-
if (!in_array($dependentPackageKey, $packageKeys, true)) {
161-
if ($this->isComposerDependency($dependentPackageKey)) {
192+
$extensionKey = $this->getPackageExtensionKey($dependentPackageKey);
193+
if (!in_array($dependentPackageKey, $packageKeys, true)
194+
&& !in_array($extensionKey, $packageKeys, true)
195+
) {
196+
if (!$this->isTypo3SystemOrCustomExtension($dependentPackageKey)) {
162197
// The given package has a dependency to a Composer package that has no relation to TYPO3
163198
// We can ignore those, when calculating the extension order
164199
continue;
@@ -169,21 +204,30 @@ protected function convertConfigurationForGraph(array $allPackageConstraints, ar
169204
1519931815
170205
);
171206
}
172-
$dependencies[$packageKey]['after'][] = $dependentPackageKey;
207+
$dependencies[$packageKey]['after'][] = $extensionKey;
173208
}
174209
}
175210
if (isset($allPackageConstraints[$packageKey]['suggestions'])) {
176211
foreach ($allPackageConstraints[$packageKey]['suggestions'] as $suggestedPackageKey) {
212+
$extensionKey = $this->getPackageExtensionKey($suggestedPackageKey);
177213
// skip suggestions on not existing packages
178-
if (in_array($suggestedPackageKey, $packageKeys, true)) {
179-
// Suggestions actually have never been meant to influence loading order.
180-
// We misuse this currently, as there is no other way to influence the loading order
181-
// for not-required packages (soft-dependency).
182-
// When considering suggestions for the loading order, we might create a cyclic dependency
183-
// if the suggested package already has a real dependency on this package, so the suggestion
184-
// has do be dropped in this case and must *not* be taken into account for loading order evaluation.
185-
$dependencies[$packageKey]['after-resilient'][] = $suggestedPackageKey;
214+
if (!in_array($suggestedPackageKey, $packageKeys, true)
215+
&& !in_array($extensionKey, $packageKeys, true)
216+
) {
217+
continue;
218+
}
219+
if (!$this->isTypo3SystemOrCustomExtension($extensionKey ?: $suggestedPackageKey)) {
220+
// Ignore non TYPO3 extension packages for suggestion determination/ordering.
221+
continue;
186222
}
223+
224+
// Suggestions actually have never been meant to influence loading order.
225+
// We misuse this currently, as there is no other way to influence the loading order
226+
// for not-required packages (soft-dependency).
227+
// When considering suggestions for the loading order, we might create a cyclic dependency
228+
// if the suggested package already has a real dependency on this package, so the suggestion
229+
// has do be dropped in this case and must *not* be taken into account for loading order evaluation.
230+
$dependencies[$packageKey]['after-resilient'][] = $extensionKey;
187231
}
188232
}
189233
}
@@ -250,25 +294,28 @@ protected function getDependencyArrayForPackage(PackageInterface $package, array
250294
foreach ($dependentPackageConstraints as $constraint) {
251295
if ($constraint instanceof PackageConstraint) {
252296
$dependentPackageKey = $constraint->getValue();
253-
if (!in_array($dependentPackageKey, $dependentPackageKeys, true) && !in_array($dependentPackageKey, $trace, true)) {
254-
$dependentPackageKeys[] = $dependentPackageKey;
297+
$extensionKey = $this->getPackageExtensionKey($dependentPackageKey) ?: $dependentPackageKey;
298+
if (!in_array($extensionKey, $dependentPackageKeys, true)) {
299+
$dependentPackageKeys[] = $extensionKey;
255300
}
256-
if (!isset($this->packages[$dependentPackageKey])) {
257-
if ($this->isComposerDependency($dependentPackageKey)) {
301+
302+
if (!isset($this->packages[$extensionKey])) {
303+
if (!$this->isTypo3SystemOrCustomExtension($extensionKey)) {
258304
// The given package has a dependency to a Composer package that has no relation to TYPO3
259305
// We can ignore those, when calculating the extension order
260306
continue;
261307
}
308+
262309
throw new Exception(
263310
sprintf(
264311
'Package "%s" depends on package "%s" which does not exist.',
265312
$package->getPackageKey(),
266-
$dependentPackageKey
313+
$extensionKey
267314
),
268315
1695119749
269316
);
270317
}
271-
$this->getDependencyArrayForPackage($this->packages[$dependentPackageKey], $dependentPackageKeys, $trace);
318+
$this->getDependencyArrayForPackage($this->packages[$extensionKey], $dependentPackageKeys, $trace);
272319
}
273320
}
274321
return array_reverse($dependentPackageKeys);
@@ -287,9 +334,17 @@ protected function getSuggestionArrayForPackage(PackageInterface $package): arra
287334
foreach ($suggestedPackageConstraints as $constraint) {
288335
if ($constraint instanceof PackageConstraint) {
289336
$suggestedPackageKey = $constraint->getValue();
290-
if (isset($this->packages[$suggestedPackageKey])) {
291-
$suggestedPackageKeys[] = $suggestedPackageKey;
337+
$extensionKey = $this->getPackageExtensionKey($suggestedPackageKey) ?: $suggestedPackageKey;
338+
if (!$this->isTypo3SystemOrCustomExtension($suggestedPackageKey)) {
339+
// Suggested packages which are not installed or not a TYPO3 extension can be skipped for
340+
// sorting when not available.
341+
continue;
292342
}
343+
if (!isset($this->packages[$extensionKey])) {
344+
// Suggested extension is not available in test system installation (not symlinked), ignore it.
345+
continue;
346+
}
347+
$suggestedPackageKeys[] = $extensionKey;
293348
}
294349
}
295350
return array_reverse($suggestedPackageKeys);
@@ -303,15 +358,38 @@ protected function findFrameworkKeys(): array
303358
$frameworkKeys = [];
304359
foreach ($this->packages as $package) {
305360
if ($package->getPackageMetaData()->isFrameworkType()) {
306-
$frameworkKeys[] = $package->getPackageKey();
361+
$frameworkKeys[] = $this->getPackageExtensionKey($package->getPackageKey()) ?: $package->getPackageKey();
307362
}
308363
}
309364
return $frameworkKeys;
310365
}
311366

312-
protected function isComposerDependency(string $packageKey): bool
367+
/**
368+
* Determines if given composer package key is either a `typo3-cms-framework` or `typo3-cms-extension` package.
369+
*/
370+
protected function isTypo3SystemOrCustomExtension(string $packageKey): bool
371+
{
372+
$packageInfo = $this->getPackageInfo($packageKey);
373+
if ($packageInfo === null) {
374+
return false;
375+
}
376+
return $packageInfo->isSystemExtension() || $packageInfo->isExtension();
377+
}
378+
379+
/**
380+
* Returns package extension key. Returns empty string if not available.
381+
*/
382+
protected function getPackageExtensionKey(string $packageKey): string
383+
{
384+
$packageInfo = $this->getPackageInfo($packageKey);
385+
if ($packageInfo === null) {
386+
return '';
387+
}
388+
return $packageInfo->getExtensionKey();
389+
}
390+
391+
protected function getPackageInfo(string $packageKey): ?PackageInfo
313392
{
314-
$packageInfo = $this->composerPackageManager->getPackageInfo($packageKey);
315-
return !(($packageInfo?->isSystemExtension() ?? false) || ($packageInfo?->isExtension()));
393+
return $this->composerPackageManager->getPackageInfo($packageKey);
316394
}
317395
}

Tests/Unit/Composer/ComposerPackageManagerTest.php

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,4 +295,67 @@ public function extensionWithJsonCanBeResolvedByRelativeLegacyPath(): void
295295
self::assertNotNull($packageInfo->getInfo());
296296
self::assertNotNull($packageInfo->getExtEmConf());
297297
}
298+
299+
public static function packagesWithoutExtEmConfFileDataProvider(): \Generator
300+
{
301+
yield 'package0 => package0' => [
302+
'path' => __DIR__ . '/../Fixtures/Packages/package0',
303+
'expectedExtensionKey' => 'package0',
304+
'expectedPackageName' => 'typo3/testing-framework-package-0',
305+
];
306+
yield 'package0 => package1' => [
307+
'path' => __DIR__ . '/../Fixtures/Packages/package1',
308+
'expectedExtensionKey' => 'package1',
309+
'expectedPackageName' => 'typo3/testing-framework-package-1',
310+
];
311+
yield 'package0 => package2' => [
312+
'path' => __DIR__ . '/../Fixtures/Packages/package2',
313+
'expectedExtensionKey' => 'package2',
314+
'expectedPackageName' => 'typo3/testing-framework-package-2',
315+
];
316+
yield 'package-identifier => some_test_extension' => [
317+
'path' => __DIR__ . '/../Fixtures/Packages/package-identifier',
318+
'expectedExtensionKey' => 'some_test_extension',
319+
'expectedPackageName' => 'typo3/testing-framework-package-identifier',
320+
];
321+
}
322+
323+
#[DataProvider('packagesWithoutExtEmConfFileDataProvider')]
324+
#[Test]
325+
public function getPackageInfoWithFallbackReturnsExtensionInfoWithCorrectExtensionKeyWhenNotHavingAnExtEmConfFile(
326+
string $path,
327+
string $expectedExtensionKey,
328+
string $expectedPackageName,
329+
): void {
330+
$packageInfo = (new ComposerPackageManager())->getPackageInfoWithFallback($path);
331+
self::assertInstanceOf(PackageInfo::class, $packageInfo, 'PackageInfo retrieved for ' . $path);
332+
self::assertNull($packageInfo->getExtEmConf(), 'Package provides ext_emconf.php');
333+
self::assertNotNull($packageInfo->getInfo(), 'Package has no composer info (composer.json)');
334+
self::assertNotEmpty($packageInfo->getInfo(), 'Package composer info is empty');
335+
self::assertTrue($packageInfo->isExtension(), 'Package is not a extension');
336+
self::assertFalse($packageInfo->isSystemExtension(), 'Package is a system extension');
337+
self::assertTrue($packageInfo->isComposerPackage(), 'Package is not a composer package');
338+
self::assertFalse($packageInfo->isMonoRepository(), 'Package is mono repository');
339+
self::assertSame($expectedPackageName, $packageInfo->getName());
340+
self::assertSame($expectedExtensionKey, $packageInfo->getExtensionKey());
341+
}
342+
343+
#[Test]
344+
public function getPackageInfoWithFallbackReturnsExtensionInfoWithCorrectExtensionKeyAndHavingAnExtEmConfFile(): void
345+
{
346+
$path = __DIR__ . '/../Fixtures/Packages/package-with-extemconf';
347+
$expectedExtensionKey = 'extension_with_extemconf';
348+
$expectedPackageName = 'typo3/testing-framework-package-with-extemconf';
349+
$packageInfo = (new ComposerPackageManager())->getPackageInfoWithFallback($path);
350+
self::assertInstanceOf(PackageInfo::class, $packageInfo, 'PackageInfo retrieved for ' . $path);
351+
self::assertNotNull($packageInfo->getExtEmConf(), 'Package has ext_emconf.php file');
352+
self::assertNotNull($packageInfo->getInfo(), 'Package has composer info');
353+
self::assertNotEmpty($packageInfo->getInfo(), 'Package composer info is not empty');
354+
self::assertTrue($packageInfo->isExtension(), 'Package is a extension');
355+
self::assertFalse($packageInfo->isSystemExtension(), 'Package is not a system extension');
356+
self::assertTrue($packageInfo->isComposerPackage(), 'Package is a composer package');
357+
self::assertFalse($packageInfo->isMonoRepository(), 'Package is not mono repository root');
358+
self::assertSame($expectedPackageName, $packageInfo->getName());
359+
self::assertSame($expectedExtensionKey, $packageInfo->getExtensionKey());
360+
}
298361
}

0 commit comments

Comments
 (0)