Skip to content
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.

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

[1.x] Add before hook support to features with names #130

Merged
merged 1 commit into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/Drivers/DatabaseDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public function getAll($features): array
$filtered = $records->where('name', $feature)->where('scope', Feature::serializeScope($scope));

if ($filtered->isNotEmpty()) {
return json_decode($filtered->value('value'), flags: JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR);
return json_decode($filtered->value('value'), flags: JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR); // @phpstan-ignore argument.type
}

return with($this->resolveValue($feature, $scope), function ($value) use ($feature, $scope, $inserts) {
Expand Down
37 changes: 33 additions & 4 deletions src/Drivers/Decorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -330,13 +330,13 @@ public function getAll($features): array
$resolvedBefore = $features->reduce(function ($resolved, $scopes, $feature) use (&$hasUnresolvedFeatures) {
$resolved[$feature] = [];

if (! method_exists($feature, 'before')) {
if (! $this->hasBeforeHook($feature)) {
$hasUnresolvedFeatures = true;

return $resolved;
}

$before = $this->container->make($feature)->before(...);
$before = $this->container->make($this->implementationClass($feature))->before(...);

foreach ($scopes as $index => $scope) {
$value = $this->resolveBeforeHook($feature, $scope, $before);
Expand Down Expand Up @@ -430,8 +430,8 @@ public function get($feature, $scope): mixed
return $item['value'];
}

$before = method_exists($feature, 'before')
? $this->container->make($feature)->before(...)
$before = $this->hasBeforeHook($feature)
? $this->container->make($this->implementationClass($feature))->before(...)
: fn () => null;

$value = $this->resolveBeforeHook($feature, $scope, $before) ?? $this->driver->get($feature, $scope);
Expand Down Expand Up @@ -682,6 +682,35 @@ protected function ensureDynamicFeatureIsDefined($feature)
});
}

/**
* Determine if the given feature has a before hook.
*
* @param string $feature
* @return bool
*/
protected function hasBeforeHook($feature)
{
$implementation = $this->implementationClass($feature);

return is_string($implementation) && class_exists($implementation) && method_exists($implementation, 'before');
}

/**
* Retrieve the implementation feature class for the given feature name.
*
* @return ?string
*/
protected function implementationClass($feature)
{
$class = $this->nameMap[$feature] ?? $feature;

if (is_string($class) && class_exists($class)) {
return $class;
}

return null;
}

/**
* Resolve the scope.
*
Expand Down
66 changes: 64 additions & 2 deletions tests/Feature/DatabaseDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1460,11 +1460,22 @@ public function test_it_can_get_features_with_before_hook()
$queries++;
});
FeatureWithBeforeHook::$before = fn ($scope) => ['before' => 'value'];
FeatureWithBeforeHookAndCustomName::$before = fn ($scope) => ['before' => 'value 2'];

$value = Feature::get(FeatureWithBeforeHook::class, null);

$this->assertSame(['before' => 'value'], $value);
$this->assertSame(0, $queries);

$value = Feature::get(FeatureWithBeforeHookAndCustomName::class, null);

$this->assertSame(['before' => 'value 2'], $value);
$this->assertSame(0, $queries);

$value = Feature::get('feature-with-before-hook-and-custom-name', null);

$this->assertSame(['before' => 'value 2'], $value);
$this->assertSame(0, $queries);
}

public function test_it_handles_null_scope_for_before_hook()
Expand All @@ -1491,6 +1502,40 @@ public function test_it_handles_null_scope_for_before_hook()
Event::assertDispatchedTimes(UnexpectedNullScopeEncountered::class, 2);
}

public function test_it_can_use_before_hook_when_using_feature_name_property()
{
$queries = 0;
FeatureWithBeforeHookAndCustomName::$before = fn ($scope) => 'before';
Feature::define(FeatureWithBeforeHookAndCustomName::class);
Feature::activate(FeatureWithBeforeHookAndCustomName::class, 'stored-value');
Feature::flushCache();
DB::listen(function (QueryExecuted $event) use (&$queries) {
$queries++;
});

$value = Feature::for(null)->value(FeatureWithBeforeHookAndCustomName::class);

$this->assertSame('before', $value);
$this->assertSame(0, $queries);
}

public function test_it_can_use_before_hook_when_using_feature_name_property_on_a_dynamically_registered_feature()
{
$queries = 0;
FeatureWithBeforeHookAndCustomName::$before = fn ($scope) => 'before';
Feature::activate(FeatureWithBeforeHookAndCustomName::class, 'stored-value');
Feature::activate('blah', 'stored-value');
Feature::flushCache();
DB::listen(function (QueryExecuted $event) use (&$queries) {
$queries++;
});

$value = Feature::for(null)->value(FeatureWithBeforeHookAndCustomName::class);

$this->assertSame('before', $value);
$this->assertSame(0, $queries);
}

public function test_it_maintains_scope_feature_keys()
{
$count = 0;
Expand Down Expand Up @@ -1571,7 +1616,7 @@ public function test_it_keys_by_feature_name()
]));
}

public function testItCanLoadAllFeaturesForScope()
public function test_it_can_load_all_features_for_scope()
{
Feature::define('bar', fn ($scope) => $scope === 'taylor');
Feature::define('foo', fn ($scope) => $scope === 'tim');
Expand Down Expand Up @@ -1602,7 +1647,7 @@ public function testItCanLoadAllFeaturesForScope()
], $records[3]);
}

public function testCanRetrieveAllFeaturesForDifferingScopeTypes(): void
public function test_can_retrieve_all_features_for_differing_scope_types(): void
{
Feature::define('user', fn (User $user) => 1);
Feature::define('nullable-user', fn (?User $user) => 2);
Expand Down Expand Up @@ -1688,6 +1733,23 @@ public function before()
}
}

class FeatureWithBeforeHookAndCustomName
{
public string $name = 'feature-with-before-hook-and-custom-name';

public static $before;

public function resolve()
{
return 'feature-value';
}

public function before()
{
return (static::$before)(...func_get_args());
}
}

class FeatureWithTypedBeforeHook
{
public static $before;
Expand Down
6 changes: 3 additions & 3 deletions tests/Feature/FeatureHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ protected function setUp(): void
Config::set('pennant.default', 'array');
}

public function testItReturnsFeatureManager()
public function test_it_returns_feature_manager()
{
$this->assertNotNull(feature());
$this->assertSame(Feature::getFacadeRoot(), feature());
}

public function testItReturnsTheFeatureValue()
public function test_it_returns_the_feature_value()
{
Feature::activate('foo', 'bar');

$this->assertSame('bar', feature('foo'));
}

public function testItConditionallyExecutesCodeBlocks()
public function test_it_conditionally_executes_code_blocks()
{
Feature::activate('foo');
$inactive = $active = null;
Expand Down
2 changes: 1 addition & 1 deletion tests/IntersectionTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ protected function setUp(): void
DB::enableQueryLog();
}

public function testCanRetrieveAllFeaturesForDifferingScopeTypes(): void
public function test_can_retrieve_all_features_for_differing_scope_types(): void
{
Feature::define('user', fn (User $user) => 1);
Feature::define('nullable-user', fn (?User $user) => 2);
Expand Down
Loading