From 8fa61f0723de2b477d9b7f72d53205f203c169e6 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 26 Feb 2025 15:22:18 -0500 Subject: [PATCH 1/5] preserve values and add restore --- ...01_add_active_column_to_features_table.php | 28 ++++ src/Contracts/Driver.php | 7 + src/Drivers/ArrayDriver.php | 15 ++ src/Drivers/DatabaseDriver.php | 138 +++++++++++++++++- src/Drivers/Decorator.php | 25 ++++ src/PendingScopedFeatureInteraction.php | 14 ++ tests/Feature/DatabaseDriverTest.php | 53 +++++++ 7 files changed, 275 insertions(+), 5 deletions(-) create mode 100644 database/migrations/2025_02_23_000001_add_active_column_to_features_table.php diff --git a/database/migrations/2025_02_23_000001_add_active_column_to_features_table.php b/database/migrations/2025_02_23_000001_add_active_column_to_features_table.php new file mode 100644 index 0000000..37ec563 --- /dev/null +++ b/database/migrations/2025_02_23_000001_add_active_column_to_features_table.php @@ -0,0 +1,28 @@ +boolean('active')->after('value')->nullable(); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('features', function (Blueprint $table) { + $table->dropColumn('active'); + }); + } +}; diff --git a/src/Contracts/Driver.php b/src/Contracts/Driver.php index f0592c2..cf26684 100644 --- a/src/Contracts/Driver.php +++ b/src/Contracts/Driver.php @@ -26,6 +26,13 @@ public function defined(): array; */ public function getAll(array $features): array; + /** + * Retrieve a feature flag's raw value, regardless if it is active. + * + * @return mixed + */ + public function getRaw(string $feature, mixed $scope); + /** * Retrieve a feature flag's value. */ diff --git a/src/Drivers/ArrayDriver.php b/src/Drivers/ArrayDriver.php index 2d7c4b7..5404865 100644 --- a/src/Drivers/ArrayDriver.php +++ b/src/Drivers/ArrayDriver.php @@ -9,7 +9,9 @@ use Laravel\Pennant\Contracts\HasFlushableCache; use Laravel\Pennant\Events\UnknownFeatureResolved; use Laravel\Pennant\Feature; +use RuntimeException; use stdClass; +use UnexpectedValueException; class ArrayDriver implements CanListStoredFeatures, Driver, HasFlushableCache { @@ -55,6 +57,19 @@ public function __construct(Dispatcher $events, $featureStateResolvers) $this->unknownFeatureValue = new stdClass; } + /** + * Retrieve a feature flag's raw value. + * + * @param string $feature + * @param mixed $scope + * + * @return mixed + */ + public function getRaw($feature, $scope) + { + throw new RuntimeException('This pennant driver does not support getting raw values.'); + } + /** * Define an initial feature flag state resolver. * diff --git a/src/Drivers/DatabaseDriver.php b/src/Drivers/DatabaseDriver.php index 432bf2a..6ff60a7 100644 --- a/src/Drivers/DatabaseDriver.php +++ b/src/Drivers/DatabaseDriver.php @@ -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); // @phpstan-ignore argument.type + return $this->value($filtered->first()); } return with($this->resolveValue($feature, $scope), function ($value) use ($feature, $scope, $inserts) { @@ -195,6 +195,59 @@ public function getAll($features): array return $results; } + /** + * Get the feature record's value + * + * @param object $record + */ + protected function value(object $record): mixed + { + $value = json_decode($record->value, flags: JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR); + + if (isset($record->active)) { + return (bool) $record->active ? $value : false; + } + + return $value; + } + + /** + * Retrieve a feature flag's raw value. + * + * @param string $feature + * @param mixed $scope + * + * @return mixed + */ + public function getRaw($feature, $scope) + { + if (($record = $this->retrieve($feature, $scope)) !== null) { + return json_decode($record->value, flags: JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR); + } + + return with($this->resolveValue($feature, $scope), function ($value) use ($feature, $scope) { + if ($value === $this->unknownFeatureValue) { + return false; + } + + try { + $this->insert($feature, $scope, $value); + } catch (UniqueConstraintViolationException $e) { + if ($this->retryDepth === 1) { + throw new RuntimeException('Unable to insert feature value into the database.', previous: $e); + } + + $this->retryDepth++; + + return $this->getRaw($feature, $scope); + } finally { + $this->retryDepth = 0; + } + + return $value; + }); + } + /** * Retrieve a feature flag's value. * @@ -204,7 +257,7 @@ public function getAll($features): array public function get($feature, $scope): mixed { if (($record = $this->retrieve($feature, $scope)) !== null) { - return json_decode($record->value, flags: JSON_OBJECT_AS_ARRAY | JSON_THROW_ON_ERROR); + return $this->value($record); } return with($this->resolveValue($feature, $scope), function ($value) use ($feature, $scope) { @@ -263,6 +316,26 @@ protected function resolveValue($feature, $scope) return $this->featureStateResolvers[$feature]($scope); } + /** + * Set a raw value for a feature flag attribute. + * + * @param string $feature + * @param mixed $scope + * @param string $key + * @param mixed $value + */ + public function setRaw($feature, $scope, $key, $value): void + { + $this->newQuery()->upsert([ + 'name' => $feature, + 'scope' => Feature::serializeScope($scope), + 'active' => false, + 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), + static::CREATED_AT => $now = Carbon::now(), + static::UPDATED_AT => $now, + ], uniqueBy: ['name', 'scope'], update: ['active', static::UPDATED_AT]); + } + /** * Set a feature flag's value. * @@ -272,13 +345,40 @@ protected function resolveValue($feature, $scope) */ public function set($feature, $scope, $value): void { + if ($value) { + $this->newQuery()->upsert([ + 'name' => $feature, + 'scope' => Feature::serializeScope($scope), + 'active' => true, + 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), + static::CREATED_AT => $now = Carbon::now(), + static::UPDATED_AT => $now, + ], uniqueBy: ['name', 'scope'], update: ['active', 'value', static::UPDATED_AT]); + + return; + } + + if (is_null($value)) { + $this->newQuery()->upsert([ + 'name' => $feature, + 'scope' => Feature::serializeScope($scope), + 'active' => true, + 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), + static::CREATED_AT => $now = Carbon::now(), + static::UPDATED_AT => $now, + ], uniqueBy: ['name', 'scope'], update: ['active', static::UPDATED_AT]); + + return; + } + $this->newQuery()->upsert([ 'name' => $feature, 'scope' => Feature::serializeScope($scope), + 'active' => false, 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), static::CREATED_AT => $now = Carbon::now(), static::UPDATED_AT => $now, - ], uniqueBy: ['name', 'scope'], update: ['value', static::UPDATED_AT]); + ], uniqueBy: ['name', 'scope'], update: ['active', static::UPDATED_AT]); } /** @@ -289,12 +389,35 @@ public function set($feature, $scope, $value): void */ public function setForAllScopes($feature, $value): void { - $this->newQuery() + if ($value) { + $this->newQuery() ->where('name', $feature) ->update([ + 'active' => true, 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), static::UPDATED_AT => Carbon::now(), ]); + + return; + } + + if (is_null($value)) { + $this->newQuery() + ->where('name', $feature) + ->update([ + 'active' => true, + static::UPDATED_AT => Carbon::now(), + ]); + + return; + } + + $this->newQuery() + ->where('name', $feature) + ->update([ + 'active' => false, + static::UPDATED_AT => Carbon::now(), + ]); } /** @@ -310,9 +433,13 @@ protected function update($feature, $scope, $value) return (bool) $this->newQuery() ->where('name', $feature) ->where('scope', Feature::serializeScope($scope)) - ->update([ + ->update(!$value ? [ + 'active' => true, 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), static::UPDATED_AT => Carbon::now(), + ]:[ + 'active' => false, + static::UPDATED_AT => Carbon::now(), ]); } @@ -347,6 +474,7 @@ protected function insertMany($inserts) 'name' => $insert['name'], 'scope' => Feature::serializeScope($insert['scope']), 'value' => json_encode($insert['value'], flags: JSON_THROW_ON_ERROR), + 'active' => $insert['value'] ? true : false, static::CREATED_AT => $now, static::UPDATED_AT => $now, ], $inserts)); diff --git a/src/Drivers/Decorator.php b/src/Drivers/Decorator.php index f7865cc..15451db 100644 --- a/src/Drivers/Decorator.php +++ b/src/Drivers/Decorator.php @@ -406,6 +406,19 @@ protected function normalizeFeaturesToLoad($features) ); } + /** + * Retrieve a feature flag's raw value. + * + * @internal + * + * @param string $feature + * @param mixed $scope + */ + public function getRaw($feature, $scope): mixed + { + return $this->driver->getRaw($feature, $scope); + } + /** * Retrieve a feature flag's value. * @@ -485,6 +498,18 @@ public function set($feature, $scope, $value): void Event::dispatch(new FeatureUpdated($feature, $scope, $value)); } + /** + * Restore the feature for everyone. + * + * @param string|array $feature + * @return void + */ + public function restoreForEveryone($feature) + { + Collection::wrap($feature) + ->each(fn ($name) => $this->setForAllScopes($name, null)); + } + /** * Activate the feature for everyone. * diff --git a/src/PendingScopedFeatureInteraction.php b/src/PendingScopedFeatureInteraction.php index 654bdeb..f39c6c7 100644 --- a/src/PendingScopedFeatureInteraction.php +++ b/src/PendingScopedFeatureInteraction.php @@ -240,6 +240,20 @@ public function unless($feature, $whenInactive, $whenActive = null) return $this->when($feature, $whenActive ?? fn () => null, $whenInactive); } + /** + * Restore the feature. + * + * @param string|array $feature + * @param mixed $fallback + * @return void + */ + public function restore($feature, $fallback = true) + { + Collection::wrap($feature) + ->crossJoin($this->scope()) + ->each(fn ($bits) => $this->driver->set($bits[0], $bits[1], $this->driver->getRaw($feature, $this->scope()[0]) ?: $fallback)); + } + /** * Activate the feature. * diff --git a/tests/Feature/DatabaseDriverTest.php b/tests/Feature/DatabaseDriverTest.php index a693eac..b9dca0f 100644 --- a/tests/Feature/DatabaseDriverTest.php +++ b/tests/Feature/DatabaseDriverTest.php @@ -55,6 +55,59 @@ public function test_it_defaults_to_false_for_unknown_values() $this->assertCount(1, DB::getQueryLog()); } + public function test_it_can_restore_a_rich_feature_value() + { + Feature::define('foo', fn () => false); + + Feature::for('tim')->activate('foo', 'bar'); + + $this->assertEquals(Feature::for('tim')->value('foo'), 'bar'); + $this->assertTrue(Feature::for('tim')->active('foo')); + $this->assertEquals(Feature::getDriver()->get('foo', 'tim'), 'bar'); + $this->assertEquals(Feature::getDriver()->getRaw('foo', 'tim'), 'bar'); + + Feature::for('tim')->deactivate('foo'); + + $this->assertFalse(Feature::for('tim')->value('foo')); + $this->assertFalse(Feature::for('tim')->active('foo')); + $this->assertFalse(Feature::getDriver()->get('foo', 'tim')); + $this->assertEquals(Feature::getDriver()->getRaw('foo', 'tim'), 'bar'); + + Feature::for('tim')->restore('foo'); + + $this->assertEquals(Feature::for('tim')->value('foo'), 'bar'); + $this->assertTrue(Feature::for('tim')->active('foo')); + $this->assertEquals(Feature::getDriver()->get('foo', 'tim'), 'bar'); + $this->assertEquals(Feature::getDriver()->getRaw('foo', 'tim'), 'bar'); + } + + public function test_it_can_restore_a_rich_feature_value_for_everyone() + { + Feature::define('foo', fn () => false); + + Feature::for('tim')->activate('foo', 'bar'); + Feature::for('taylor')->activate('foo', 'bar'); + + $this->assertEquals(Feature::for('tim')->value('foo'), 'bar'); + $this->assertEquals(Feature::for('taylor')->value('foo'), 'bar'); + $this->assertEquals(Feature::getDriver()->get('foo', 'tim'), 'bar'); + $this->assertEquals(Feature::getDriver()->get('foo', 'taylor'), 'bar'); + + Feature::deactivateForEveryone('foo'); + + $this->assertFalse(Feature::for('tim')->value('foo')); + $this->assertFalse(Feature::for('taylor')->value('foo')); + $this->assertFalse(Feature::getDriver()->get('foo', 'tim')); + $this->assertFalse(Feature::getDriver()->get('foo', 'taylor')); + + Feature::restoreForEveryone('foo'); + + $this->assertEquals(Feature::for('tim')->value('foo'), 'bar'); + $this->assertEquals(Feature::for('taylor')->value('foo'), 'bar'); + $this->assertEquals(Feature::getDriver()->get('foo', 'tim'), 'bar'); + $this->assertEquals(Feature::getDriver()->get('foo', 'taylor'), 'bar'); + } + public function test_it_dispatches_events_on_unknown_feature_checks() { Event::fake([UnknownFeatureResolved::class]); From 94ce20fc1260da8cc82999c58cf58037ab52b199 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 26 Feb 2025 15:25:57 -0500 Subject: [PATCH 2/5] wip --- src/Contracts/Driver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Contracts/Driver.php b/src/Contracts/Driver.php index cf26684..4e60d26 100644 --- a/src/Contracts/Driver.php +++ b/src/Contracts/Driver.php @@ -27,7 +27,7 @@ public function defined(): array; public function getAll(array $features): array; /** - * Retrieve a feature flag's raw value, regardless if it is active. + * Retrieve a feature flag's raw value, regardless of activation. * * @return mixed */ From 26880dd425db23f617570f5a183e6227fd457e59 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 26 Feb 2025 20:04:32 -0500 Subject: [PATCH 3/5] test fallback --- src/Drivers/DatabaseDriver.php | 2 +- tests/Feature/DatabaseDriverTest.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Drivers/DatabaseDriver.php b/src/Drivers/DatabaseDriver.php index 6ff60a7..a652db4 100644 --- a/src/Drivers/DatabaseDriver.php +++ b/src/Drivers/DatabaseDriver.php @@ -433,7 +433,7 @@ protected function update($feature, $scope, $value) return (bool) $this->newQuery() ->where('name', $feature) ->where('scope', Feature::serializeScope($scope)) - ->update(!$value ? [ + ->update($value ? [ 'active' => true, 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), static::UPDATED_AT => Carbon::now(), diff --git a/tests/Feature/DatabaseDriverTest.php b/tests/Feature/DatabaseDriverTest.php index b9dca0f..84b1c13 100644 --- a/tests/Feature/DatabaseDriverTest.php +++ b/tests/Feature/DatabaseDriverTest.php @@ -55,6 +55,23 @@ public function test_it_defaults_to_false_for_unknown_values() $this->assertCount(1, DB::getQueryLog()); } + public function test_it_can_restore_a_rich_feature_value_with_a_fallback() + { + Feature::define('foo', fn () => false); + + $this->assertFalse(Feature::for('tim')->value('foo')); + $this->assertFalse(Feature::for('tim')->active('foo')); + $this->assertFalse(Feature::getDriver()->get('foo', 'tim')); + $this->assertFalse(Feature::getDriver()->getRaw('foo', 'tim')); + + Feature::for('tim')->restore('foo', fallback: 'bar'); + + $this->assertEquals(Feature::for('tim')->value('foo'), 'bar'); + $this->assertTrue(Feature::for('tim')->active('foo')); + $this->assertEquals(Feature::getDriver()->get('foo', 'tim'), 'bar'); + $this->assertEquals(Feature::getDriver()->getRaw('foo', 'tim'), 'bar'); + } + public function test_it_can_restore_a_rich_feature_value() { Feature::define('foo', fn () => false); From 76bf3916cf1132a641860d02bf6a4ca7fbb4d372 Mon Sep 17 00:00:00 2001 From: = Date: Thu, 27 Feb 2025 23:25:23 -0500 Subject: [PATCH 4/5] cleanup code --- src/Drivers/DatabaseDriver.php | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/Drivers/DatabaseDriver.php b/src/Drivers/DatabaseDriver.php index a652db4..2d158a9 100644 --- a/src/Drivers/DatabaseDriver.php +++ b/src/Drivers/DatabaseDriver.php @@ -316,26 +316,6 @@ protected function resolveValue($feature, $scope) return $this->featureStateResolvers[$feature]($scope); } - /** - * Set a raw value for a feature flag attribute. - * - * @param string $feature - * @param mixed $scope - * @param string $key - * @param mixed $value - */ - public function setRaw($feature, $scope, $key, $value): void - { - $this->newQuery()->upsert([ - 'name' => $feature, - 'scope' => Feature::serializeScope($scope), - 'active' => false, - 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), - static::CREATED_AT => $now = Carbon::now(), - static::UPDATED_AT => $now, - ], uniqueBy: ['name', 'scope'], update: ['active', static::UPDATED_AT]); - } - /** * Set a feature flag's value. * From d2870e7342d1cced25cb41c32580477832516ad9 Mon Sep 17 00:00:00 2001 From: = Date: Thu, 27 Feb 2025 23:28:26 -0500 Subject: [PATCH 5/5] cleanup code --- src/Drivers/DatabaseDriver.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/Drivers/DatabaseDriver.php b/src/Drivers/DatabaseDriver.php index 2d158a9..cdc3f35 100644 --- a/src/Drivers/DatabaseDriver.php +++ b/src/Drivers/DatabaseDriver.php @@ -338,19 +338,6 @@ public function set($feature, $scope, $value): void return; } - if (is_null($value)) { - $this->newQuery()->upsert([ - 'name' => $feature, - 'scope' => Feature::serializeScope($scope), - 'active' => true, - 'value' => json_encode($value, flags: JSON_THROW_ON_ERROR), - static::CREATED_AT => $now = Carbon::now(), - static::UPDATED_AT => $now, - ], uniqueBy: ['name', 'scope'], update: ['active', static::UPDATED_AT]); - - return; - } - $this->newQuery()->upsert([ 'name' => $feature, 'scope' => Feature::serializeScope($scope),