From e944ccc8317389ae164e82daf31a8a28bf57ce6c Mon Sep 17 00:00:00 2001 From: Tim MacDonald Date: Mon, 8 Jul 2024 13:26:25 +1000 Subject: [PATCH] Introduce before hook --- .gitignore | 4 +- phpunit.xml.dist | 10 ++- src/Drivers/DatabaseDriver.php | 4 +- src/Drivers/Decorator.php | 71 +++++++++++++++--- tests/Feature/DatabaseDriverTest.php | 105 +++++++++++++++++++++++++++ 5 files changed, 178 insertions(+), 16 deletions(-) diff --git a/.gitignore b/.gitignore index 660fc15..800ab77 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ /vendor -composer.lock +/composer.lock /phpunit.xml -.phpunit.result.cache +/.phpunit.cache diff --git a/phpunit.xml.dist b/phpunit.xml.dist index a30bee0..b5b979b 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,15 +1,19 @@ - + ./tests/Feature - + ./src - + diff --git a/src/Drivers/DatabaseDriver.php b/src/Drivers/DatabaseDriver.php index 0e92052..e9b27ff 100644 --- a/src/Drivers/DatabaseDriver.php +++ b/src/Drivers/DatabaseDriver.php @@ -144,7 +144,7 @@ public function getAll($features): array { $query = $this->newQuery(); - $features = Collection::make($features) + $resolved = Collection::make($features) ->map(fn ($scopes, $feature) => Collection::make($scopes) ->each(fn ($scope) => $query->orWhere( fn ($q) => $q->where('name', $feature)->where('scope', Feature::serializeScope($scope)) @@ -154,7 +154,7 @@ public function getAll($features): array $inserts = new Collection; - $results = $features->map(fn ($scopes, $feature) => $scopes->map(function ($scope) use ($feature, $records, $inserts) { + $results = $resolved->map(fn ($scopes, $feature) => $scopes->map(function ($scope) use ($feature, $records, $inserts) { $filtered = $records->where('name', $feature)->where('scope', Feature::serializeScope($scope)); if ($filtered->isNotEmpty()) { diff --git a/src/Drivers/Decorator.php b/src/Drivers/Decorator.php index a8b490e..a46ec46 100644 --- a/src/Drivers/Decorator.php +++ b/src/Drivers/Decorator.php @@ -223,12 +223,48 @@ public function getAll($features): array return []; } - return tap($this->driver->getAll($features->all()), function ($results) use ($features) { - $features->flatMap(fn ($scopes, $key) => Collection::make($scopes) - ->zip($results[$key]) - ->map(fn ($scopes) => $scopes->push($key))) - ->each(fn ($value) => $this->putInCache($value[2], $value[0], $value[1])); + $hasUnresolvedFeatures = false; + + $resolvedBefore = $features->reduce(function ($resolved, $scopes, $feature) use (&$hasUnresolvedFeatures) { + $resolved[$feature] = []; + + if (! method_exists($feature, 'before')) { + $hasUnresolvedFeatures = true; + + return $resolved; + } + + $before = $this->container->make($feature)->before(...); + + foreach ($scopes as $index => $scope) { + $value = $this->resolveBeforeHook($feature, $scope, $before); + + if ($value !== null) { + $resolved[$feature][$index] = $value; + } else { + $hasUnresolvedFeatures = true; + } + + return $resolved; + } + }, []); + + $pendingFeatures = $features->map(function ($scopes, $feature) use ($resolvedBefore) { + return array_diff_key($scopes, $resolvedBefore[$feature]); }); + + $results = array_replace_recursive( + $features->all(), + $resolvedBefore, + $hasUnresolvedFeatures ? $this->driver->getAll($pendingFeatures->all()) : [], + ); + + $features->flatMap(fn ($scopes, $key) => Collection::make($scopes) + ->zip($results[$key]) + ->map(fn ($scopes) => $scopes->push($key))) + ->each(fn ($value) => $this->putInCache($value[2], $value[0], $value[1])); + + return $results; } /** @@ -294,11 +330,28 @@ public function get($feature, $scope): mixed return $item['value']; } - return tap($this->driver->get($feature, $scope), function ($value) use ($feature, $scope) { - $this->putInCache($feature, $scope, $value); + $before = method_exists($feature, 'before') + ? $this->container->make($feature)->before(...) + : fn () => null; - Event::dispatch(new FeatureRetrieved($feature, $scope, $value)); - }); + $value = $this->resolveBeforeHook($feature, $scope, $before) ?? $this->driver->get($feature, $scope); + + $this->putInCache($feature, $scope, $value); + + Event::dispatch(new FeatureRetrieved($feature, $scope, $value)); + + return $value; + } + + protected function resolveBeforeHook($feature, $scope, $hook) + { + if ($scope === null && ! $this->canHandleNullScope($hook)) { + Event::dispatch(new UnexpectedNullScopeEncountered($feature)); + + return null; + } + + return $hook($scope); } /** diff --git a/tests/Feature/DatabaseDriverTest.php b/tests/Feature/DatabaseDriverTest.php index 764006f..961ae94 100644 --- a/tests/Feature/DatabaseDriverTest.php +++ b/tests/Feature/DatabaseDriverTest.php @@ -21,6 +21,7 @@ use Laravel\Pennant\Events\FeaturesPurged; use Laravel\Pennant\Events\FeatureUpdated; use Laravel\Pennant\Events\FeatureUpdatedForAllScopes; +use Laravel\Pennant\Events\UnexpectedNullScopeEncountered; use Laravel\Pennant\Events\UnknownFeatureResolved; use Laravel\Pennant\Feature; use RuntimeException; @@ -1382,6 +1383,80 @@ public function test_it_only_retries_on_conflicts_for_individual_queries() $this->assertSame(1, $insertAttempts); } + + public function test_it_can_intercept_feature_values_using_before_hook() + { + $queries = 0; + FeatureWithBeforeHook::$before = fn ($scope) => 'before'; + Feature::define(FeatureWithBeforeHook::class); + Feature::activate(FeatureWithBeforeHook::class, 'stored-value'); + Feature::flushCache(); + DB::listen(function (QueryExecuted $event) use (&$queries) { + $queries++; + }); + + $value = Feature::for(null)->value(FeatureWithBeforeHook::class); + + $this->assertSame('before', $value); + $this->assertSame(0, $queries); + } + + public function test_it_can_merges_before_with_resolved_features() + { + $queries = 0; + DB::listen(function (QueryExecuted $event) use (&$queries) { + $queries++; + }); + FeatureWithBeforeHook::$before = fn ($scope) => ['before' => 'value']; + Feature::define('foo', 'bar'); + Feature::define(FeatureWithBeforeHook::class); + + $values = Feature::for('user:1')->all(); + + $this->assertSame([ + 'foo' => 'bar', + 'Tests\Feature\FeatureWithBeforeHook' => ['before' => 'value'], + ], $values); + $this->assertSame(2, $queries); + } + + public function test_it_can_get_features_with_before_hook() + { + $queries = 0; + DB::listen(function (QueryExecuted $event) use (&$queries) { + $queries++; + }); + FeatureWithBeforeHook::$before = fn ($scope) => ['before' => 'value']; + + $value = Feature::get(FeatureWithBeforeHook::class, null); + + $this->assertSame(['before' => 'value'], $value); + $this->assertSame(0, $queries); + } + + public function test_it_handles_null_scope_for_before_hook() + { + Event::fake(UnexpectedNullScopeEncountered::class); + $queries = 0; + DB::listen(function (QueryExecuted $event) use (&$queries) { + $queries++; + }); + FeatureWithTypedBeforeHook::$before = fn ($scope) => 'before-value'; + Feature::define(FeatureWithTypedBeforeHook::class); + + $values = Feature::for(null)->value(FeatureWithTypedBeforeHook::class); + + $this->assertSame('feature-value', $values); + $this->assertSame(2, $queries); + + Feature::flushCache(); + + $value = Feature::get(FeatureWithTypedBeforeHook::class, null); + + $this->assertSame('feature-value', $values); + $this->assertSame(3, $queries); + Event::assertDispatchedTimes(UnexpectedNullScopeEncountered::class, 2); + } } class UnregisteredFeature @@ -1409,3 +1484,33 @@ public function __invoke() return 'unregistered-value'; } } + +class FeatureWithBeforeHook +{ + public static $before; + + public function resolve() + { + return 'feature-value'; + } + + public function before() + { + return (static::$before)(...func_get_args()); + } +} + +class FeatureWithTypedBeforeHook +{ + public static $before; + + public function resolve() + { + return 'feature-value'; + } + + public function before(string $scope) + { + return (static::$before)(...func_get_args()); + } +}