diff --git a/CHANGELOG.md b/CHANGELOG.md index a965f58..8399c1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The `API` service is now an isolated `OpenFeatureAPI` instance created by the container (SDK 2.3.0 isolated instances) instead of the `OpenFeatureAPI::getInstance()` global singleton. Provider, hooks, and evaluation context are no longer shared with other kernels running in the same PHP process. - `open-feature/sdk` requirement raised from `^2.2` to `^2.3` (the bundle relies on the public `OpenFeatureAPI` constructor introduced in SDK 2.3.0). +### Fixed + +- `flags` and `providers`: keys are now kept as declared. Dashes were converted to underscores (a flag declared as `new-checkout` could only be evaluated as `new_checkout`), and an object flag holding a `name` key was renamed after that value. The undocumented list form `flags: [{name: ..., value: ...}]` is no longer supported. + ### Upgrade notes - Run `composer update open-feature/sdk` if your lock file pins a version below 2.3.0. diff --git a/src/OpenFeatureBundle.php b/src/OpenFeatureBundle.php index f8656a5..ff52ce1 100644 --- a/src/OpenFeatureBundle.php +++ b/src/OpenFeatureBundle.php @@ -42,6 +42,7 @@ public function configure(DefinitionConfigurator $definition): void ->arrayNode('providers') ->info('Multiple providers combined through the SDK MultiProvider. Keys are provider names, values are service IDs. Evaluation follows declaration order.') ->useAttributeAsKey('name') + ->normalizeKeys(false) ->scalarPrototype()->end() ->end() ->arrayNode('strategy') @@ -64,7 +65,8 @@ public function configure(DefinitionConfigurator $definition): void ->end() ->arrayNode('flags') ->info('Flag values for the built-in InMemoryProvider (local/dev use).') - ->useAttributeAsKey('name') + // No key attribute: it would re-key object flags holding that key + ->normalizeKeys(false) ->variablePrototype()->end() ->end() ->arrayNode('redis') diff --git a/tests/DependencyInjection/Compiler/RegisterHooksPassTest.php b/tests/DependencyInjection/Compiler/RegisterHooksPassTest.php new file mode 100644 index 0000000..7d2f8f1 --- /dev/null +++ b/tests/DependencyInjection/Compiler/RegisterHooksPassTest.php @@ -0,0 +1,47 @@ +register(API::class, OpenFeatureAPI::class); + $container->register('app.hook', ContextRecordingHook::class)->addTag('openfeature.hook'); + $container->register('app.other_hook', ContextRecordingHook::class)->addTag('openfeature.hook'); + $container->register('app.untagged_hook', ContextRecordingHook::class); + + (new RegisterHooksPass())->process($container); + + $this->assertEquals( + [ + ['addHooks', [new Reference('app.hook')]], + ['addHooks', [new Reference('app.other_hook')]], + ], + $container->getDefinition(API::class)->getMethodCalls(), + ); + } + + public function testDoesNothingWithoutTheApiService(): void + { + $container = new ContainerBuilder(); + $container->register('app.hook', ContextRecordingHook::class)->addTag('openfeature.hook'); + + $this->expectNotToPerformAssertions(); + + (new RegisterHooksPass())->process($container); + } +} diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php new file mode 100644 index 0000000..12c7fe0 --- /dev/null +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -0,0 +1,116 @@ +assertSame([ + 'provider' => null, + 'providers' => [], + 'strategy' => ['type' => 'first_match', 'fallback' => null], + 'flags' => [], + 'evaluation_context' => ['user_provider' => 'auto'], + 'feature_flag' => ['on_disabled' => 'auto', 'status_code' => 403], + ], $this->process([])); + } + + public function testStrategyShorthandSetsTheType(): void + { + $config = $this->process(['strategy' => 'first_successful']); + + $this->assertSame(['type' => 'first_successful', 'fallback' => null], $config['strategy']); + } + + public function testProviderNamesAreNotNormalized(): void + { + $config = $this->process(['providers' => ['my-local' => 'app.provider']]); + + $this->assertSame(['my-local' => 'app.provider'], $config['providers']); + } + + public function testFlagKeysAreNotNormalized(): void + { + $config = $this->process(['flags' => ['new-checkout' => true]]); + + $this->assertSame(['new-checkout' => true], $config['flags']); + } + + public function testObjectFlagWithANameKeyIsKeptAsIs(): void + { + $config = $this->process(['flags' => ['banner' => ['name' => 'Summer sale', 'color' => 'red']]]); + + $this->assertSame(['banner' => ['name' => 'Summer sale', 'color' => 'red']], $config['flags']); + } + + public function testFlagsAreMergedPerKeyAcrossConfigs(): void + { + $config = $this->process( + ['flags' => ['dark_mode' => false, 'banner' => ['color' => 'red']]], + ['flags' => ['banner' => ['size' => 'xl']]], + ); + + $this->assertSame(['dark_mode' => false, 'banner' => ['size' => 'xl']], $config['flags']); + } + + #[DataProvider('provideUserProviderValues')] + public function testUserProviderAcceptsBooleans(bool|string $value, string $expected): void + { + $config = $this->process(['evaluation_context' => ['user_provider' => $value]]); + + $this->assertSame(['user_provider' => $expected], $config['evaluation_context']); + } + + /** @return array */ + public static function provideUserProviderValues(): array + { + return [ + 'true' => [true, 'true'], + 'false' => [false, 'false'], + 'auto' => ['auto', 'auto'], + ]; + } + + public function testRedisPrefixHasADefault(): void + { + $config = $this->process(['redis' => ['client' => 'app.redis']]); + + $this->assertSame(['client' => 'app.redis', 'prefix' => 'feature:'], $config['redis']); + } + + public function testStatusCodeIsRejectedWithAccessDenied(): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid configuration for path "open_feature.feature_flag": "status_code" has no effect when "on_disabled" is "access_denied".'); + + $this->process(['feature_flag' => ['on_disabled' => 'access_denied', 'status_code' => 404]]); + } + + /** + * @param array ...$configs + * + * @return array + */ + private function process(array ...$configs): array + { + /** @var array $processed */ + $processed = (new Processor())->processConfiguration(new Configuration(new OpenFeatureBundle(), null, 'open_feature'), $configs); + + return $processed; + } +} diff --git a/tests/Fixtures/ContextRecordingHook.php b/tests/Fixtures/ContextRecordingHook.php new file mode 100644 index 0000000..bea92dc --- /dev/null +++ b/tests/Fixtures/ContextRecordingHook.php @@ -0,0 +1,42 @@ +context = $context->getEvaluationContext(); + } + + public function error(HookContext $context, \Throwable $error, HookHints $hints): void + { + } + + public function finally(HookContext $context, HookHints $hints): void + { + } + + public function supportsFlagValueType(string $flagValueType): bool + { + return true; + } +} diff --git a/tests/Profiler/ContextProviderRecorderTest.php b/tests/Profiler/ContextProviderRecorderTest.php new file mode 100644 index 0000000..d119474 --- /dev/null +++ b/tests/Profiler/ContextProviderRecorderTest.php @@ -0,0 +1,43 @@ +createStub(EvaluationContextProviderInterface::class); + + $recorder = new ContextProviderRecorder(); + $recorder(new EvaluationContextContributedEvent($provider, new MutableEvaluationContext('user-1'))); + $recorder(new EvaluationContextContributedEvent($provider, new MutableEvaluationContext(null, new MutableAttributes(['plan' => 'premium'])))); + + $this->assertSame([ + ['provider' => $provider::class, 'targeting_key' => 'user-1', 'attributes' => []], + ['provider' => $provider::class, 'targeting_key' => null, 'attributes' => ['plan' => 'premium']], + ], $recorder->getContributions()); + } + + public function testResetClearsContributions(): void + { + $provider = $this->createStub(EvaluationContextProviderInterface::class); + + $recorder = new ContextProviderRecorder(); + $recorder(new EvaluationContextContributedEvent($provider, new MutableEvaluationContext('user-1'))); + $this->assertNotEmpty($recorder->getContributions()); + + $recorder->reset(); + $this->assertSame([], $recorder->getContributions()); + } +} diff --git a/tests/Profiler/ProfilerHookTest.php b/tests/Profiler/ProfilerHookTest.php new file mode 100644 index 0000000..b36facf --- /dev/null +++ b/tests/Profiler/ProfilerHookTest.php @@ -0,0 +1,111 @@ +assertSame([], $hook->getEvaluations()); + } + + public function testAfterRecordsEvaluation(): void + { + $hook = new ProfilerHook(); + $details = (new ResolutionDetailsBuilder())->withValue(true)->withReason('STATIC')->withVariant('on')->build(); + + $hook->after($this->hookContext('my_flag'), $details, new HookHints()); + + $this->assertSame([ + ['flag' => 'my_flag', 'type' => FlagValueType::BOOLEAN, 'value' => true, 'variant' => 'on', 'reason' => 'STATIC', 'error' => null], + ], $hook->getEvaluations()); + } + + public function testErrorRecordsEvaluationWithErrorReason(): void + { + $hook = new ProfilerHook(); + + $hook->error($this->hookContext('broken_flag'), new \RuntimeException('Provider unavailable'), new HookHints()); + + $this->assertSame([ + ['flag' => 'broken_flag', 'type' => FlagValueType::BOOLEAN, 'value' => null, 'variant' => null, 'reason' => 'ERROR', 'error' => 'RuntimeException: Provider unavailable'], + ], $hook->getEvaluations()); + } + + public function testAfterRecordsErrorMessageFromResolutionError(): void + { + $hook = new ProfilerHook(); + $details = (new ResolutionDetailsBuilder()) + ->withValue(false) + ->withError(new ResolutionError(ErrorCode::FLAG_NOT_FOUND(), 'flag not found')) + ->build(); + + $hook->after($this->hookContext('my_flag'), $details, new HookHints()); + + $this->assertSame('flag not found', $hook->getEvaluations()[0]['error']); + } + + #[DataProvider('provideFlagValueTypes')] + public function testSupportsEveryFlagValueType(string $type): void + { + $this->assertTrue((new ProfilerHook())->supportsFlagValueType($type)); + } + + /** @return iterable */ + public static function provideFlagValueTypes(): iterable + { + foreach ([FlagValueType::BOOLEAN, FlagValueType::STRING, FlagValueType::INTEGER, FlagValueType::FLOAT, FlagValueType::OBJECT] as $type) { + yield $type => [$type]; + } + } + + public function testBeforeReturnsNull(): void + { + $this->assertNull((new ProfilerHook())->before($this->hookContext('my_flag'), new HookHints())); + } + + public function testAccumulatesMultipleEvaluations(): void + { + $hook = new ProfilerHook(); + $details = (new ResolutionDetailsBuilder())->withValue(true)->build(); + + foreach (['flag_a', 'flag_b', 'flag_c'] as $flagKey) { + $hook->after($this->hookContext($flagKey), $details, new HookHints()); + } + + $this->assertSame(['flag_a', 'flag_b', 'flag_c'], \array_column($hook->getEvaluations(), 'flag')); + } + + public function testResetClearsEvaluations(): void + { + $hook = new ProfilerHook(); + $hook->error($this->hookContext('my_flag'), new \RuntimeException('boom'), new HookHints()); + $this->assertNotEmpty($hook->getEvaluations()); + + $hook->reset(); + + $this->assertSame([], $hook->getEvaluations()); + } + + private function hookContext(string $flagKey): HookContext + { + return (new HookContextBuilder())->withFlagKey($flagKey)->withType(FlagValueType::BOOLEAN)->build(); + } +}