Skip to content

Commit 438102e

Browse files
kayw-geekkayw-geek
authored
Fix workers() not respecting user-specified order (#112)
* Fix workers() not respecting user-specified order (#111)
1 parent d848183 commit 438102e

7 files changed

Lines changed: 191 additions & 19 deletions

File tree

‎packages-tests/Release/DisableDefaultWorkers/DisableDefaultWorkersTest.php‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,18 @@
44

55
namespace Symplify\MonorepoBuilder\Tests\Release\DisableDefaultWorkers;
66

7+
use Symplify\MonorepoBuilder\Release\Contract\ReleaseWorker\ReleaseWorkerInterface;
78
use PHPUnit\Framework\TestCase;
89
use ReflectionClass;
910
use Symplify\MonorepoBuilder\Config\MBConfig;
1011
use Symplify\MonorepoBuilder\Kernel\MonorepoBuilderKernel;
12+
use Symplify\MonorepoBuilder\Release\ReleaseWorker\AddTagToChangelogReleaseWorker;
1113
use Symplify\MonorepoBuilder\Release\ReleaseWorker\PushTagReleaseWorker;
1214
use Symplify\MonorepoBuilder\Release\ReleaseWorker\TagVersionReleaseWorker;
15+
use Symplify\MonorepoBuilder\Release\ReleaseWorkerProvider;
16+
use Symplify\MonorepoBuilder\Release\ValueObject\Stage;
17+
use Symplify\MonorepoBuilder\Tests\Release\DisableDefaultWorkers\Fixture\FetchTagsReleaseWorker;
18+
use Symplify\MonorepoBuilder\Tests\Release\DisableDefaultWorkers\Fixture\GenerateChangelogReleaseWorker;
1319

1420
/**
1521
* Tests for MBConfig::disableDefaultWorkers() functionality.
@@ -20,13 +26,11 @@ final class DisableDefaultWorkersTest extends TestCase
2026
{
2127
protected function setUp(): void
2228
{
23-
// Reset static state before each test
2429
$this->resetMBConfigState();
2530
}
2631

2732
protected function tearDown(): void
2833
{
29-
// Clean up static state after each test
3034
$this->resetMBConfigState();
3135
}
3236

@@ -62,16 +66,70 @@ public function testDisableDefaultWorkersPreservesUserAddedWorker(): void
6266
$monorepoBuilderKernel = new MonorepoBuilderKernel();
6367
$container = $monorepoBuilderKernel->createFromConfigs([__DIR__ . '/config/disable_and_add_custom.php']);
6468

65-
// User explicitly added TagVersionReleaseWorker, so it should be preserved
66-
$this->assertTrue($container->has(TagVersionReleaseWorker::class));
6769
// User did not add PushTagReleaseWorker, so it should be removed
6870
$this->assertFalse($container->has(PushTagReleaseWorker::class));
71+
72+
// User explicitly added TagVersionReleaseWorker via workers(), so it should be available
73+
/** @var ReleaseWorkerProvider $provider */
74+
$provider = $container->get(ReleaseWorkerProvider::class);
75+
$workers = $provider->provideByStage(Stage::MAIN);
76+
$workerClasses = array_map(static fn (ReleaseWorkerInterface $releaseWorker): string => $releaseWorker::class, $workers);
77+
$this->assertContains(TagVersionReleaseWorker::class, $workerClasses);
78+
}
79+
80+
/**
81+
* Scenario 4: workers() without disableDefaultWorkers() should not duplicate overlapping workers
82+
*/
83+
public function testWorkersWithoutDisableDoesNotDuplicate(): void
84+
{
85+
$monorepoBuilderKernel = new MonorepoBuilderKernel();
86+
$container = $monorepoBuilderKernel->createFromConfigs([__DIR__ . '/config/add_custom_without_disable.php']);
87+
88+
/** @var ReleaseWorkerProvider $provider */
89+
$provider = $container->get(ReleaseWorkerProvider::class);
90+
$workers = $provider->provideByStage(Stage::MAIN);
91+
$workerClasses = array_map(static fn (ReleaseWorkerInterface $releaseWorker): string => $releaseWorker::class, $workers);
92+
93+
// Should have exactly 3 workers, no duplicates
94+
$this->assertSame([
95+
AddTagToChangelogReleaseWorker::class,
96+
TagVersionReleaseWorker::class,
97+
PushTagReleaseWorker::class,
98+
], $workerClasses);
99+
}
100+
101+
/**
102+
* Scenario 5: Reproduces the exact scenario from issue #111.
103+
* User wants custom workers to run BEFORE the default tag/push workers.
104+
*
105+
* @see https://github.com/symplify/monorepo-builder/issues/111
106+
*/
107+
public function testWorkerOrderIsRespected(): void
108+
{
109+
$monorepoBuilderKernel = new MonorepoBuilderKernel();
110+
$container = $monorepoBuilderKernel->createFromConfigs([__DIR__ . '/config/disable_and_reorder.php']);
111+
112+
/** @var ReleaseWorkerProvider $provider */
113+
$provider = $container->get(ReleaseWorkerProvider::class);
114+
$workers = $provider->provideByStage(Stage::MAIN);
115+
$workerClasses = array_map(static fn (ReleaseWorkerInterface $releaseWorker): string => $releaseWorker::class, $workers);
116+
117+
$this->assertSame([
118+
FetchTagsReleaseWorker::class,
119+
GenerateChangelogReleaseWorker::class,
120+
TagVersionReleaseWorker::class,
121+
PushTagReleaseWorker::class,
122+
], $workerClasses);
69123
}
70124

71125
private function resetMBConfigState(): void
72126
{
73127
$reflectionClass = new ReflectionClass(MBConfig::class);
128+
74129
$reflectionProperty = $reflectionClass->getProperty('disableDefaultWorkers');
75130
$reflectionProperty->setValue(null, false);
131+
132+
$reflectionProperty = $reflectionClass->getProperty('userWorkerClasses');
133+
$reflectionProperty->setValue(null, []);
76134
}
77135
}

‎packages-tests/Release/DisableDefaultWorkers/Fixture/FetchTagsReleaseWorker.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Symplify\MonorepoBuilder\Tests\Release\DisableDefaultWorkers\Fixture;
6+
7+
use PharIo\Version\Version;
8+
use Symplify\MonorepoBuilder\Release\Contract\ReleaseWorker\ReleaseWorkerInterface;
9+
10+
final readonly class FetchTagsReleaseWorker implements ReleaseWorkerInterface
11+
{
12+
public function work(Version $version): void
13+
{
14+
}
15+
16+
public function getDescription(Version $version): string
17+
{
18+
return 'Fetched remote tags';
19+
}
20+
}

‎packages-tests/Release/DisableDefaultWorkers/Fixture/GenerateChangelogReleaseWorker.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Symplify\MonorepoBuilder\Tests\Release\DisableDefaultWorkers\Fixture;
6+
7+
use PharIo\Version\Version;
8+
use Symplify\MonorepoBuilder\Release\Contract\ReleaseWorker\ReleaseWorkerInterface;
9+
10+
final readonly class GenerateChangelogReleaseWorker implements ReleaseWorkerInterface
11+
{
12+
public function work(Version $version): void
13+
{
14+
}
15+
16+
public function getDescription(Version $version): string
17+
{
18+
return 'Generate changelog from conventional commits';
19+
}
20+
}

‎packages-tests/Release/DisableDefaultWorkers/config/add_custom_without_disable.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Symplify\MonorepoBuilder\Config\MBConfig;
6+
use Symplify\MonorepoBuilder\Release\ReleaseWorker\AddTagToChangelogReleaseWorker;
7+
use Symplify\MonorepoBuilder\Release\ReleaseWorker\PushTagReleaseWorker;
8+
use Symplify\MonorepoBuilder\Release\ReleaseWorker\TagVersionReleaseWorker;
9+
10+
return static function (MBConfig $mbConfig): void {
11+
// User adds workers without disabling defaults, including overlap with defaults
12+
$mbConfig->workers([
13+
AddTagToChangelogReleaseWorker::class,
14+
TagVersionReleaseWorker::class,
15+
PushTagReleaseWorker::class,
16+
]);
17+
};

‎packages-tests/Release/DisableDefaultWorkers/config/disable_and_reorder.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Symplify\MonorepoBuilder\Config\MBConfig;
6+
use Symplify\MonorepoBuilder\Release\ReleaseWorker\PushTagReleaseWorker;
7+
use Symplify\MonorepoBuilder\Release\ReleaseWorker\TagVersionReleaseWorker;
8+
use Symplify\MonorepoBuilder\Tests\Release\DisableDefaultWorkers\Fixture\FetchTagsReleaseWorker;
9+
use Symplify\MonorepoBuilder\Tests\Release\DisableDefaultWorkers\Fixture\GenerateChangelogReleaseWorker;
10+
11+
/**
12+
* Reproduces the exact scenario from issue #111:
13+
* User wants custom workers to run BEFORE the default tag/push workers.
14+
*
15+
* @see https://github.com/symplify/monorepo-builder/issues/111
16+
*/
17+
return static function (MBConfig $mbConfig): void {
18+
MBConfig::disableDefaultWorkers();
19+
20+
$mbConfig->workers([
21+
FetchTagsReleaseWorker::class,
22+
GenerateChangelogReleaseWorker::class,
23+
TagVersionReleaseWorker::class,
24+
PushTagReleaseWorker::class,
25+
]);
26+
};

‎src/Config/MBConfig.php‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ final class MBConfig extends ContainerConfigurator
1313
{
1414
private static bool $disableDefaultWorkers = false;
1515

16+
/**
17+
* @var array<class-string<ReleaseWorkerInterface>>
18+
*/
19+
private static array $userWorkerClasses = [];
20+
1621
/**
1722
* @param string[] $packageDirectories
1823
*/
@@ -29,6 +34,14 @@ public static function isDisableDefaultWorkers(): bool
2934
return self::$disableDefaultWorkers;
3035
}
3136

37+
/**
38+
* @return array<class-string<ReleaseWorkerInterface>>
39+
*/
40+
public static function getUserWorkerClasses(): array
41+
{
42+
return self::$userWorkerClasses;
43+
}
44+
3245
public static function disableDefaultWorkers(): void
3346
{
3447
self::$disableDefaultWorkers = true;
@@ -74,8 +87,13 @@ public function workers(array $workerClasses): void
7487
{
7588
$services = $this->services();
7689

77-
foreach ($workerClasses as $workerClass) {
78-
$services->set($workerClass);
90+
self::$userWorkerClasses = $workerClasses;
91+
92+
// Use a unique service ID prefix so user-registered workers don't replace
93+
// default definitions (which would preserve the original array position in
94+
// the container and break the user's intended ordering).
95+
foreach ($workerClasses as $index => $workerClass) {
96+
$services->set('user_release_worker.' . $index, $workerClass);
7997
}
8098
}
8199

‎src/DependencyInjection/CompilerPass/RemoveDefaultWorkersCompilerPass.php‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,32 +9,45 @@
99
use Symplify\MonorepoBuilder\Config\MBConfig;
1010

1111
/**
12-
* Removes default release workers from the container if MBConfig::disableDefaultWorkers() was called.
12+
* Manages default release workers based on user configuration:
1313
*
14-
* This uses a tag-based approach to distinguish between:
15-
* - Default workers registered by config/config.php (tagged with 'monorepo.default_worker')
16-
* - User-registered workers (no tag, or re-registered without the tag)
17-
*
18-
* When user calls disableDefaultWorkers() and then manually registers a worker
19-
* (even one with the same class as a default worker), only the tagged default
20-
* definition is removed, preserving the user's explicit registration.
14+
* 1. If disableDefaultWorkers() was called, removes all services tagged with 'monorepo.default_worker'.
15+
* 2. If workers() was called (without disableDefaultWorkers()), removes default workers whose class
16+
* overlaps with user-registered workers to prevent duplicates while preserving user-specified order.
2117
*/
2218
final readonly class RemoveDefaultWorkersCompilerPass implements CompilerPassInterface
2319
{
2420
private const DEFAULT_WORKER_TAG = 'monorepo.default_worker';
2521

2622
public function process(ContainerBuilder $containerBuilder): void
2723
{
28-
// Check if user disabled default workers in their config
29-
if (! MBConfig::isDisableDefaultWorkers()) {
24+
$taggedServices = $containerBuilder->findTaggedServiceIds(self::DEFAULT_WORKER_TAG);
25+
26+
if (MBConfig::isDisableDefaultWorkers()) {
27+
// Remove all default workers
28+
foreach (array_keys($taggedServices) as $serviceId) {
29+
if ($containerBuilder->hasDefinition($serviceId)) {
30+
$containerBuilder->removeDefinition($serviceId);
31+
}
32+
}
33+
3034
return;
3135
}
3236

33-
// Find and remove only services tagged as default workers
34-
$taggedServices = $containerBuilder->findTaggedServiceIds(self::DEFAULT_WORKER_TAG);
37+
// Remove default workers whose class was also registered by the user
38+
// via workers(), to avoid duplicates
39+
$userWorkerClasses = MBConfig::getUserWorkerClasses();
40+
if ($userWorkerClasses === []) {
41+
return;
42+
}
3543

3644
foreach (array_keys($taggedServices) as $serviceId) {
37-
if ($containerBuilder->hasDefinition($serviceId)) {
45+
if (! $containerBuilder->hasDefinition($serviceId)) {
46+
continue;
47+
}
48+
49+
$class = $containerBuilder->getDefinition($serviceId)->getClass() ?? $serviceId;
50+
if (in_array($class, $userWorkerClasses, true)) {
3851
$containerBuilder->removeDefinition($serviceId);
3952
}
4053
}

0 commit comments

Comments
 (0)