Skip to content

Remove support for deprecated default modules #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Remove support for deprecated default modules #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b1e7b4
Package // remove deprecated $defaultModules from boot()-method.
Chrico Aug 29, 2024
9da3a20
Merge branch 'master' into feature/remove-deprecated-default-modules
tfrommen Jan 8, 2025
f337a99
Merge branch 'master' into feature/remove-deprecated-default-modules-tf
tfrommen Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions docs/Application-flow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,3 @@ The steps listed above for the two stages represent the "happy paths". If any ex
2. The **`Package::ACTION_FAILED_BOOT`** action hook is fired, passing the raised `Throwable` as an argument.
3. If the `Package`'s `Properties` instance is in "debug mode" (`Properties::isDebug()` returns `true`), the exception bubbles up, and the flow stops here.
4. `Package::boot()` returns false.



## About modules passed to `Package::boot()`

Passing modules to add to `Package::boot()` has been deprecated since Modularity `v1.7.0`.

For backward compatibility, when that happens, a deprecation notice is triggered (similarly to WordPress' `_deprecated_argument`) but modules are still added.

It must be noted, that when first calling `Package::build()` and after that `Package::boot()` passing modules as argument, we will add those modules _after_ the status is already at `Package::STATUS_INITIALIZED` (because of the `Package::build()` call) and, as mentioned above, that should not be possible.

The `Package` class still deals with this scenario aiming for 100% backward compatibility, but there's an edge case. If anything that listens to the `Package::ACTION_INITIALIZED` hook accesses the container (which is an accepted and documented possibility) the compiled container will be created, which means we can't add modules to it anymore. In this specific case, calling something like `$package->build()->boot($someModule)` will end-up in an exception.

While this is a breakage of the backward compatibility promise, it is also true that `Package::build()` was introduced in `v1.7.0` when passing modules to `Package::boot()` was deprecated. Developers who have introduced `Package::build()` should also have removed any module passed to `Package::boot()`.
2 changes: 1 addition & 1 deletion docs/Package.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,4 +335,4 @@ Retrieve the current status of the application. The following statuses are avail
| `Package::STATUS_BOOTING` | Before `Package::boot()` started processing executable modules' "run procedures". |
| `Package::STATUS_BOOTED` | After `Package::boot()` ended processing executable modules' "run procedures". |
| `Package::STATUS_DONE` | The application has successfully completed both processes. |
| `Package::STATUS_FAILED` | The application did not build/boot properly. |
| `Package::STATUS_FAILED` | The application did not build/boot properly. |
91 changes: 4 additions & 87 deletions src/Package.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,10 +373,9 @@ public function build(): Package
}

/**
* @param Module ...$defaultModules Deprecated, use `addModule()` to add default modules.
* @return bool
*/
public function boot(Module ...$defaultModules): bool
public function boot(): bool
{
try {
// When package is done, nothing should happen to it calling boot again, but we call
Expand All @@ -385,9 +384,9 @@ public function boot(Module ...$defaultModules): bool
return false;
}

// Call build() if not called yet, and ensure any new module passed here is added
// as well, throwing if the container was already built.
$this->doBuild(...$defaultModules);
if (!$this->built && $this->statusIs(self::STATUS_IDLE)) {
$this->build();
}

// Make sure we call boot() on a non-failed instance, and also make a sanity check
// on the status flow, e.g. prevent calling boot() from an action hook.
Expand Down Expand Up @@ -420,88 +419,6 @@ public function boot(Module ...$defaultModules): bool
return true;
}

/**
* @param Module ...$defaultModules
* @return void
*/
private function doBuild(Module ...$defaultModules): void
{
if ($defaultModules) {
$this->deprecatedArgument(
sprintf(
'Passing default modules to %1$s::boot() is deprecated since version 1.7.0.'
. ' Please add modules via %1$s::addModule().',
__CLASS__
),
__METHOD__,
'1.7.0'
);
}

// We expect `boot()` to be called either:
// 1. Directly after `addModule()`/`connect()`, without any `build()` call in between, so
// status is IDLE and `$this->built` is `false`.
// 2. After `build()` is called, so status is INITIALIZED and `$this->built` is `true`.
// Any other usage is not allowed (e.g. calling `boot()` from an hook callback) and in that
// case we return here, giving back control to `boot()` which will throw.
$validFlows = (!$this->built && $this->statusIs(self::STATUS_IDLE))
|| ($this->built && $this->statusIs(self::STATUS_INITIALIZED));

if (!$validFlows) {
// If none of the two supported flows happened, we just return handling control back
// to `boot()`, that will throw.
return;
}

if (!$this->built) {
// First valid flow: `boot()` was called directly after `addModule()`/`connect()`
// without any call to `build()`. We can call `build()` and return, handing control
// back to `boot()`. Before returning, if we had default modules passed to `boot()` we
// already have fired a deprecation, so here we just add them dealing with back-compat.
foreach ($defaultModules as $defaultModule) {
$this->addModule($defaultModule);
}
$this->build();

return;
}

// Second valid flow: we have called `boot()` after `build()`. If we did it correctly,
// without default modules passed to `boot()`, we can just return handing control back
// to `boot()`.
if (!$defaultModules) {
return;
}

// If here, we have done something like: `$package->build()->boot($module1, $module2)`.
// Passing modules to `boot()` was deprecated when `build()` was introduced, so whoever
// added `build()` should have removed modules passed to `boot()`.
// But we want to keep 100% backward compatibility so we still support this behavior
// until the next major is released. To do that, we simulate IDLE status to prevent
// `addModule()` from throwing when adding default modules.
// But we can do that only if we don't have a compiled container yet.
// If anything hooking ACTION_INIT called `container()` we have a compiled container
// already, and we can't add modules, so we not going to simulate INIT status, which mean
// the `$this->addModule()` call below will throw.
$backup = $this->status;
try {
if (!$this->hasContainer()) {
$this->status = self::STATUS_IDLE;
}
foreach ($defaultModules as $defaultModule) {
// If a module was already added via `addModule()` we can skip it, reducing the
// chances of throwing an exception if not needed.
if (!$this->moduleIs($defaultModule->id(), self::MODULE_ADDED)) {
$this->addModule($defaultModule);
}
}
} finally {
if (!$this->hasFailed()) {
$this->status = $backup;
}
}
}

/**
* @param Module $module
* @param string $status
Expand Down
154 changes: 50 additions & 104 deletions tests/unit/PackageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,54 +401,30 @@ public function testBootWithExecutableModuleFailed(): void

/**
* @test
* @runInSeparateProcess
*/
public function testBootPassingModulesEmitDeprecation(): void
{
$module1 = $this->stubModule('module_1', ServiceModule::class);
$module1->allows('services')->andReturn($this->stubServices('service_1'));

$package = Package::new($this->stubProperties('test', true));

$this->convertDeprecationsToExceptions();
try {
$count = 0;
$package->boot($module1);
} catch (\Throwable $throwable) {
$count++;
$this->assertThrowableMessageMatches($throwable, 'boot().+?deprecated.+?1\.7');
} finally {
static::assertSame(1, $count);
}
}

/**
* @test
* @runInSeparateProcess
*/
public function testBootPassingModulesAddModules(): void
public function testAddModuleFailsAfterBuild(): void
{
$module1 = $this->stubModule('module_1', ServiceModule::class);
$module1->allows('services')->andReturn($this->stubServices('service_1'));
$module1->expects('services')->andReturn($this->stubServices('service_1'));

$package = Package::new($this->stubProperties('test', true));
$module2 = $this->stubModule('module_2', ServiceModule::class);
$module2->expects('services')->andReturn($this->stubServices('service_2'));

$this->ignoreDeprecations();
$package->boot($module1);
$module3 = $this->stubModule('module_3', ServiceModule::class);
$module3->allows('services')->andReturn($this->stubServices('service_3'));

static::assertSame('service_1', $package->container()->get('service_1')['id']);
}
$package = Package::new($this->stubProperties('test', true))
->addModule($module1)
->addModule($module2)
->build();

/**
* @test
*/
public function testAddModuleFailsAfterBuild(): void
{
$package = Package::new($this->stubProperties('test', true))->build();
$container = $package->container();

$this->expectExceptionMessageMatches("/add module/i");
static::assertSame('service_1', $container->get('service_1')['id']);
static::assertSame('service_2', $container->get('service_2')['id']);

$package->addModule($this->stubModule());
$this->expectExceptionMessageMatches("/can't add module module_3/i");
$package->addModule($module3);
}

/**
Expand Down Expand Up @@ -520,64 +496,6 @@ public function run(ContainerInterface $container): bool
static::assertSame('Works!', $actual);
}

/**
* @test
*/
public function testBuildPassingModulesToBoot(): void
{
$module1 = $this->stubModule('module_1', ServiceModule::class);
$module1->expects('services')->andReturn($this->stubServices('service_1'));

$module2 = $this->stubModule('module_2', ServiceModule::class);
$module2->expects('services')->andReturn($this->stubServices('service_2'));

$module3 = $this->stubModule('module_3', ServiceModule::class);
$module3->expects('services')->andReturn($this->stubServices('service_3'));

$package = Package::new($this->stubProperties('test', true))
->addModule($module1)
->addModule($module2)
->build();

$this->ignoreDeprecations();
$package->boot($module2, $module3);

$container = $package->container();

static::assertSame('service_1', $container->get('service_1')['id']);
static::assertSame('service_2', $container->get('service_2')['id']);
static::assertSame('service_3', $container->get('service_3')['id']);
}

/**
* @test
*/
public function testBootFailsIfPassingNotAddedModulesAfterContainer(): void
{
$module1 = $this->stubModule('module_1', ServiceModule::class);
$module1->expects('services')->andReturn($this->stubServices('service_1'));

$module2 = $this->stubModule('module_2', ServiceModule::class);
$module2->expects('services')->andReturn($this->stubServices('service_2'));

$module3 = $this->stubModule('module_3', ServiceModule::class);
$module3->allows('services')->andReturn($this->stubServices('service_3'));

$package = Package::new($this->stubProperties('test', true))
->addModule($module1)
->addModule($module2)
->build();

$container = $package->container();

static::assertSame('service_1', $container->get('service_1')['id']);
static::assertSame('service_2', $container->get('service_2')['id']);

$this->expectExceptionMessageMatches("/can't add module module_3/i");
$this->ignoreDeprecations();
$package->boot($module2, $module3);
}

/**
* @test
*/
Expand Down Expand Up @@ -773,7 +691,11 @@ public function testItFailsWhenCallingBootFromInitHookDebugOff(): void

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INIT))
->once()
->whenHappen([$package, 'boot']);
->whenHappen(
static function () use ($package): void {
$package->boot();
}
);

Monkey\Actions\expectDone(Package::ACTION_MODULARITY_INIT)->never();
Monkey\Actions\expectDone($package->hookName(Package::ACTION_INITIALIZED))->never();
Expand All @@ -793,7 +715,11 @@ public function testItFailsWhenCallingBootFromInitHookDebugOn(): void

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INIT))
->once()
->whenHappen([$package, 'boot']);
->whenHappen(
static function () use ($package): void {
$package->boot();
}
);

Monkey\Actions\expectDone(Package::ACTION_MODULARITY_INIT)->never();
Monkey\Actions\expectDone($package->hookName(Package::ACTION_INITIALIZED))->never();
Expand All @@ -814,7 +740,11 @@ public function testItFailsWhenCallingBootFromInitializedHook(): void

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INITIALIZED))
->once()
->whenHappen([$package, 'boot']);
->whenHappen(
static function () use ($package): void {
$package->boot();
}
);

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INIT))->once();
Monkey\Actions\expectDone(Package::ACTION_MODULARITY_INIT)->once();
Expand All @@ -835,7 +765,11 @@ public function testItFailsWhenCallingBootFromReadyHook(): void

Monkey\Actions\expectDone($package->hookName(Package::ACTION_BOOTED))
->once()
->whenHappen([$package, 'boot']);
->whenHappen(
static function () use ($package): void {
$package->boot();
}
);

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INIT))->once();
Monkey\Actions\expectDone(Package::ACTION_MODULARITY_INIT)->once();
Expand All @@ -856,7 +790,11 @@ public function testItFailsWhenCallingBuildFromInitHook(): void

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INIT))
->once()
->whenHappen([$package, 'build']);
->whenHappen(
static function () use ($package): void {
$package->build();
}
);

Monkey\Actions\expectDone(Package::ACTION_MODULARITY_INIT)->never();
Monkey\Actions\expectDone($package->hookName(Package::ACTION_INITIALIZED))->never();
Expand All @@ -877,7 +815,11 @@ public function testItFailsWhenCallingBuildFromInitializedHook(): void

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INITIALIZED))
->once()
->whenHappen([$package, 'build']);
->whenHappen(
static function () use ($package): void {
$package->build();
}
);

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INIT))->once();
Monkey\Actions\expectDone(Package::ACTION_MODULARITY_INIT)->once();
Expand All @@ -898,7 +840,11 @@ public function testItFailsWhenCallingBuildFromReadyHook(): void

Monkey\Actions\expectDone($package->hookName(Package::ACTION_BOOTED))
->once()
->whenHappen([$package, 'build']);
->whenHappen(
static function () use ($package): void {
$package->build();
}
);

Monkey\Actions\expectDone($package->hookName(Package::ACTION_INIT))->once();
Monkey\Actions\expectDone(Package::ACTION_MODULARITY_INIT)->once();
Expand Down
Loading