From c3fadbf6b1f021855c7938aff44a090510aa9db3 Mon Sep 17 00:00:00 2001 From: IanM <16573496+imorland@users.noreply.github.com> Date: Wed, 18 Oct 2023 19:36:59 +0100 Subject: [PATCH] [1.x] Conditional extender instantiation (#3898) * chore: create tests to highlight the conditional instantiation problem * Apply fixes from StyleCI * add callback and invokable class + tests * Apply fixes from StyleCI * address stan issue on php 8.2 * Revert "address stan issue on php 8.2" This reverts commit 1fc2c8801a2b75f462251be8ae266e1699497f95. * attempt to make stan happy * Revert "attempt to make stan happy" This reverts commit 1cc327bb3b1cc52273b18488f9cc926f43d8858e. * is it really that simple? * Revert "is it really that simple?" This reverts commit 2006755cf17157e653c610e222377cac5f127e50. * let's try this * Update framework/core/src/Extend/Conditional.php Co-authored-by: Sami Mazouz --------- Co-authored-by: StyleCI Bot Co-authored-by: Sami Mazouz --- framework/core/src/Extend/Conditional.php | 48 ++++++- .../integration/extenders/ConditionalTest.php | 136 ++++++++++++++++++ 2 files changed, 177 insertions(+), 7 deletions(-) diff --git a/framework/core/src/Extend/Conditional.php b/framework/core/src/Extend/Conditional.php index b9bbc764af..d0c1d699fe 100644 --- a/framework/core/src/Extend/Conditional.php +++ b/framework/core/src/Extend/Conditional.php @@ -13,17 +13,34 @@ use Flarum\Extension\ExtensionManager; use Illuminate\Contracts\Container\Container; +/** + * The Conditional extender allows developers to conditionally apply other extenders + * based on either boolean values or results from callable functions. + * + * This is useful for applying extenders only if certain conditions are met, + * such as the presence of an enabled extension or a specific configuration setting. + */ class Conditional implements ExtenderInterface { /** - * @var array + * An array of conditions and their associated extenders. + * + * Each entry should have: + * - 'condition': a boolean or callable that should return a boolean. + * - 'extenders': an array of extenders, a callable returning an array of extenders, or an invokable class string. + * + * @var array */ protected $conditions = []; /** - * @param ExtenderInterface[] $extenders + * Apply extenders only if a specific extension is enabled. + * + * @param string $extensionId The ID of the extension. + * @param ExtenderInterface[]|callable|string $extenders An array of extenders, a callable returning an array of extenders, or an invokable class string. + * @return self */ - public function whenExtensionEnabled(string $extensionId, array $extenders): self + public function whenExtensionEnabled(string $extensionId, $extenders): self { return $this->when(function (ExtensionManager $extensions) use ($extensionId) { return $extensions->isEnabled($extensionId); @@ -31,10 +48,14 @@ public function whenExtensionEnabled(string $extensionId, array $extenders): sel } /** - * @param bool|callable $condition - * @param ExtenderInterface[] $extenders + * Apply extenders based on a condition. + * + * @param bool|callable $condition A boolean or callable that should return a boolean. + * If this evaluates to true, the extenders will be applied. + * @param ExtenderInterface[]|callable|string $extenders An array of extenders, a callable returning an array of extenders, or an invokable class string. + * @return self */ - public function when($condition, array $extenders): self + public function when($condition, $extenders): self { $this->conditions[] = [ 'condition' => $condition, @@ -44,6 +65,13 @@ public function when($condition, array $extenders): self return $this; } + /** + * Iterates over the conditions and applies the associated extenders if the conditions are met. + * + * @param Container $container + * @param Extension|null $extension + * @return void + */ public function extend(Container $container, Extension $extension = null) { foreach ($this->conditions as $condition) { @@ -52,7 +80,13 @@ public function extend(Container $container, Extension $extension = null) } if ($condition['condition']) { - foreach ($condition['extenders'] as $extender) { + $extenders = $condition['extenders']; + + if (is_string($extenders) || is_callable($extenders)) { + $extenders = $container->call($extenders); + } + + foreach ($extenders as $extender) { $extender->extend($container, $extension); } } diff --git a/framework/core/tests/integration/extenders/ConditionalTest.php b/framework/core/tests/integration/extenders/ConditionalTest.php index ec96423e1f..2ee1512073 100644 --- a/framework/core/tests/integration/extenders/ConditionalTest.php +++ b/framework/core/tests/integration/extenders/ConditionalTest.php @@ -159,4 +159,140 @@ public function conditional_injects_dependencies_to_condition_callable() $this->app(); } + + /** @test */ + public function conditional_does_not_instantiate_extender_if_condition_is_false_using_callable() + { + $this->extend( + (new Extend\Conditional()) + ->when(false, TestExtender::class) + ); + + $this->app(); + + $response = $this->send( + $this->request('GET', '/api', [ + 'authenticatedAs' => 1, + ]) + ); + + $payload = json_decode($response->getBody()->getContents(), true); + + $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']); + } + + /** @test */ + public function conditional_does_instantiate_extender_if_condition_is_true_using_callable() + { + $this->extend( + (new Extend\Conditional()) + ->when(true, TestExtender::class) + ); + + $this->app(); + + $response = $this->send( + $this->request('GET', '/api', [ + 'authenticatedAs' => 1, + ]) + ); + + $payload = json_decode($response->getBody()->getContents(), true); + + $this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']); + } + + /** @test */ + public function conditional_does_not_instantiate_extender_if_condition_is_false_using_callback() + { + $this->extend( + (new Extend\Conditional()) + ->when(false, function (): array { + return [ + (new Extend\ApiSerializer(ForumSerializer::class)) + ->attributes(function () { + return [ + 'customConditionalAttribute' => true + ]; + }) + ]; + }) + ); + + $this->app(); + + $response = $this->send( + $this->request('GET', '/api', [ + 'authenticatedAs' => 1, + ]) + ); + + $payload = json_decode($response->getBody()->getContents(), true); + + $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']); + } + + /** @test */ + public function conditional_does_instantiate_extender_if_condition_is_true_using_callback() + { + $this->extend( + (new Extend\Conditional()) + ->when(true, function (): array { + return [ + (new Extend\ApiSerializer(ForumSerializer::class)) + ->attributes(function () { + return [ + 'customConditionalAttribute' => true + ]; + }) + ]; + }) + ); + + $this->app(); + + $response = $this->send( + $this->request('GET', '/api', [ + 'authenticatedAs' => 1, + ]) + ); + + $payload = json_decode($response->getBody()->getContents(), true); + + $this->assertArrayHasKey('customConditionalAttribute', $payload['data']['attributes']); + } + + /** @test */ + public function conditional_does_not_work_if_extension_is_disabled() + { + $this->extend( + (new Extend\Conditional()) + ->whenExtensionEnabled('dummy-extension-id', TestExtender::class) + ); + + $response = $this->send( + $this->request('GET', '/api', [ + 'authenticatedAs' => 1, + ]) + ); + + $payload = json_decode($response->getBody()->getContents(), true); + + $this->assertArrayNotHasKey('customConditionalAttribute', $payload['data']['attributes']); + } +} + +class TestExtender +{ + public function __invoke(): array + { + return [ + (new Extend\ApiSerializer(ForumSerializer::class)) + ->attributes(function () { + return [ + 'customConditionalAttribute' => true + ]; + }) + ]; + } }