fix: keep flags and providers config keys as declared, version the test suite

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.

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')

tests/DependencyInjection/Compiler/RegisterHooksPassTest.php

@@ -0,0 +1,47 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Aubes\OpenFeatureBundle\Tests\DependencyInjection\Compiler;
+
+use Aubes\OpenFeatureBundle\DependencyInjection\Compiler\RegisterHooksPass;
+use Aubes\OpenFeatureBundle\Tests\Fixtures\ContextRecordingHook;
+use OpenFeature\interfaces\flags\API;
+use OpenFeature\OpenFeatureAPI;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Reference;
+
+#[CoversClass(RegisterHooksPass::class)]
+class RegisterHooksPassTest extends TestCase
+{
+    public function testAddsTaggedHooksToTheApi(): void
+    {
+        $container = new ContainerBuilder();
+        $container->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);
+    }
+}

tests/DependencyInjection/ConfigurationTest.php

@@ -0,0 +1,116 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Aubes\OpenFeatureBundle\Tests\DependencyInjection;
+
+use Aubes\OpenFeatureBundle\OpenFeatureBundle;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Config\Definition\Configuration;
+use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
+use Symfony\Component\Config\Definition\Processor;
+
+/**
+ * Configuration tree only: cross-field validations run in loadExtension() and are covered by OpenFeatureBundleTest.
+ */
+#[CoversClass(OpenFeatureBundle::class)]
+class ConfigurationTest extends TestCase
+{
+    public function testDefaultConfiguration(): void
+    {
+        $this->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<string, array{bool|string, string}> */
+    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<string, mixed> ...$configs
+     *
+     * @return array<string, mixed>
+     */
+    private function process(array ...$configs): array
+    {
+        /** @var array<string, mixed> $processed */
+        $processed = (new Processor())->processConfiguration(new Configuration(new OpenFeatureBundle(), null, 'open_feature'), $configs);
+
+        return $processed;
+    }
+}

tests/Fixtures/ContextRecordingHook.php

@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Aubes\OpenFeatureBundle\Tests\Fixtures;
+
+use OpenFeature\interfaces\flags\EvaluationContext;
+use OpenFeature\interfaces\hooks\Hook;
+use OpenFeature\interfaces\hooks\HookContext;
+use OpenFeature\interfaces\hooks\HookHints;
+use OpenFeature\interfaces\provider\ResolutionDetails;
+
+/**
+ * Records the evaluation context seen by the last flag evaluation.
+ */
+class ContextRecordingHook implements Hook
+{
+    public ?EvaluationContext $context = null;
+
+    public function before(HookContext $context, HookHints $hints): ?EvaluationContext
+    {
+        return null;
+    }
+
+    public function after(HookContext $context, ResolutionDetails $details, HookHints $hints): void
+    {
+        $this->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;
+    }
+}

tests/Profiler/ContextProviderRecorderTest.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Aubes\OpenFeatureBundle\Tests\Profiler;
+
+use Aubes\OpenFeatureBundle\EvaluationContext\EvaluationContextProviderInterface;
+use Aubes\OpenFeatureBundle\Event\EvaluationContextContributedEvent;
+use Aubes\OpenFeatureBundle\Profiler\ContextProviderRecorder;
+use OpenFeature\implementation\flags\MutableAttributes;
+use OpenFeature\implementation\flags\MutableEvaluationContext;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(ContextProviderRecorder::class)]
+class ContextProviderRecorderTest extends TestCase
+{
+    public function testRecordsEachContributionInOrder(): void
+    {
+        $provider = $this->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());
+    }
+}

tests/Profiler/ProfilerHookTest.php

@@ -0,0 +1,111 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Aubes\OpenFeatureBundle\Tests\Profiler;
+
+use Aubes\OpenFeatureBundle\Profiler\ProfilerHook;
+use OpenFeature\implementation\hooks\HookContextBuilder;
+use OpenFeature\implementation\hooks\HookHints;
+use OpenFeature\implementation\provider\ResolutionDetailsBuilder;
+use OpenFeature\implementation\provider\ResolutionError;
+use OpenFeature\interfaces\flags\FlagValueType;
+use OpenFeature\interfaces\hooks\HookContext;
+use OpenFeature\interfaces\provider\ErrorCode;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(ProfilerHook::class)]
+class ProfilerHookTest extends TestCase
+{
+    public function testStartsWithNoEvaluations(): void
+    {
+        $hook = new ProfilerHook();
+
+        $this->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<string, array{string}> */
+    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();
+    }
+}