Merge pull request #70 from Sybio/2.x-allow-feature-annotation-to-be-…

…repeatable feat: allow Feature annotation to be repeatable
novaway · Aug 28, 2024 · a29b63c · a29b63c
2 parents 996ee64 + 5084082
commit a29b63c
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 4 deletions.
diff --git a/src/Annotation/Feature.php b/src/Annotation/Feature.php
@@ -15,7 +15,7 @@
  * @Annotation
  * @NamedArgumentConstructor()
  */
-#[\Attribute]
+#[\Attribute(\Attribute::TARGET_ALL | \Attribute::IS_REPEATABLE)]
 class Feature
 {
     /** @var string */

diff --git a/src/DataCollector/FeatureCollector.php b/src/DataCollector/FeatureCollector.php
@@ -32,7 +32,7 @@ public function __construct(FeatureManager $manager, StorageInterface $storage)
         $this->storage = $storage;
     }
 
-    public function collect(Request $request, Response $response, \Throwable $exception = null): void
+    public function collect(Request $request, Response $response, ?\Throwable $exception = null): void
     {
         $features = $this->storage->all();
         $activeFeatureCount = count(

diff --git a/src/Model/Feature.php b/src/Model/Feature.php
@@ -26,7 +26,7 @@ class Feature implements FeatureInterface
     /**
      * Constructor
      */
-    public function __construct(string $key, bool $enabled, string $description = null)
+    public function __construct(string $key, bool $enabled, ?string $description = null)
     {
         $this->key = $key;
         $this->enabled = $enabled;

diff --git a/tests/Fixtures/App/TestBundle/Controller/DefaultController.php b/tests/Fixtures/App/TestBundle/Controller/DefaultController.php
@@ -62,6 +62,24 @@ public function annotationFooDisabled()
         return new Response('DefaultController::annotationFooDisabledAction');
     }
 
+    /**
+     * @Feature("foo", enabled=true)
+     * @Feature("bar", enabled=true)
+     */
+    public function annotationFooEnabledBarEnabled()
+    {
+        return new Response('DefaultController::annotationFooEnabledBarEnabledAction');
+    }
+
+    /**
+     * @Feature("foo", enabled=true)
+     * @Feature("bar", enabled=false)
+     */
+    public function annotationFooEnabledBarDisabled()
+    {
+        return new Response('DefaultController::annotationFooEnabledBarDisabledAction');
+    }
+
     #[Feature(name: 'foo')]
     public function attributeFooEnabled()
     {
@@ -73,4 +91,18 @@ public function attributeFooDisabled()
     {
         return new Response('DefaultController::attributeFooDisabledAction');
     }
+
+    #[Feature(name: 'foo', enabled: true)]
+    #[Feature(name: 'bar', enabled: true)]
+    public function attributeFooEnabledBarEnabled()
+    {
+        return new Response('DefaultController::attributeFooEnabledBarEnabledAction');
+    }
+
+    #[Feature(name: 'foo', enabled: true)]
+    #[Feature(name: 'bar', enabled: false)]
+    public function attributeFooEnabledBarDisabled()
+    {
+        return new Response('DefaultController::attributeFooEnabledBarDisabledAction');
+    }
 }
diff --git a/tests/Fixtures/App/config/routing.yml b/tests/Fixtures/App/config/routing.yml
@@ -32,6 +32,15 @@ annotation_configuration_disabled:
     path:     /annotation/disabled
     defaults: { _controller: Novaway\Bundle\FeatureFlagBundle\Tests\Fixtures\App\TestBundle\Controller\DefaultController::annotationFooDisabled }
 
+annotation_configuration_bar_enabled_foo_enabled:
+    path:     /annotation/bar/enabled/foo/enabled
+    defaults: { _controller: Novaway\Bundle\FeatureFlagBundle\Tests\Fixtures\App\TestBundle\Controller\DefaultController::annotationFooEnabledBarEnabled }
+
+annotation_configuration_bar_enabled_foo_disabled:
+    path:     /annotation/bar/enabled/foo/disabled
+    defaults: { _controller: Novaway\Bundle\FeatureFlagBundle\Tests\Fixtures\App\TestBundle\Controller\DefaultController::annotationFooEnabledBarDisabled }
+
+
 attribute_class_disabled:
     path:     /attribute/class/disabled
     defaults: { _controller:  Novaway\Bundle\FeatureFlagBundle\Tests\Fixtures\App\TestBundle\Controller\AttributeClassDisabledController }
@@ -43,3 +52,11 @@ attribute_configuration_enabled:
 attribute_configuration_disabled:
     path:     /attribute/disabled
     defaults: { _controller: Novaway\Bundle\FeatureFlagBundle\Tests\Fixtures\App\TestBundle\Controller\DefaultController::attributeFooDisabled }
+
+attribute_configuration_bar_enabled_foo_enabled:
+    path:     /attribute/bar/enabled/foo/enabled
+    defaults: { _controller: Novaway\Bundle\FeatureFlagBundle\Tests\Fixtures\App\TestBundle\Controller\DefaultController::attributeFooEnabledBarEnabled }
+
+attribute_configuration_bar_enabled_foo_disabled:
+    path:     /attribute/bar/enabled/foo/disabled
+    defaults: { _controller: Novaway\Bundle\FeatureFlagBundle\Tests\Fixtures\App\TestBundle\Controller\DefaultController::attributeFooEnabledBarDisabled }
diff --git a/tests/Functional/Controller/DefaultControllerTest.php b/tests/Functional/Controller/DefaultControllerTest.php
@@ -83,6 +83,30 @@ public function testAnnotationFooDisabledAction()
         static::assertSame(404, static::$client->getResponse()->getStatusCode());
     }
 
+    /**
+     * @requires PHP >= 8.0
+     */
+    public function testTwoAnnotationsFooEnabledBarEnabledHasNoAccessBecauseFeatureFooIsEnabledButFeatureBarIsDisabled()
+    {
+        static::$client->request('GET', '/annotation/bar/enabled/foo/enabled');
+
+        static::assertSame(404, static::$client->getResponse()->getStatusCode());
+    }
+
+    /**
+     * @requires PHP >= 8.0
+     */
+    public function testTwoAnnotationsFooEnabledBarDisabledHasAccessBecauseFeatureFooIsEnabledAndFeatureBarIsDisabled()
+    {
+        $crawler = static::$client->request('GET', '/annotation/bar/enabled/foo/disabled');
+
+        static::assertSame(200, static::$client->getResponse()->getStatusCode());
+        static::assertGreaterThan(
+            0,
+            $crawler->filter('html:contains("DefaultController::annotationFooEnabledBarDisabledAction")')->count(),
+        );
+    }
+
     /**
      * @requires PHP >= 8.0
      */
@@ -106,4 +130,28 @@ public function testAttributeFooDisabledAction()
 
         static::assertSame(404, static::$client->getResponse()->getStatusCode());
     }
+
+    /**
+     * @requires PHP >= 8.0
+     */
+    public function testTwoAttributesFooEnabledBarEnabledHasNoAccessBecauseFeatureFooIsEnabledButFeatureBarIsDisabled()
+    {
+        static::$client->request('GET', '/attribute/bar/enabled/foo/enabled');
+
+        static::assertSame(404, static::$client->getResponse()->getStatusCode());
+    }
+
+    /**
+     * @requires PHP >= 8.0
+     */
+    public function testTwoAttributesFooEnabledBarDisabledHasAccessBecauseFeatureFooIsEnabledAndFeatureBarIsDisabled()
+    {
+        $crawler = static::$client->request('GET', '/attribute/bar/enabled/foo/disabled');
+
+        static::assertSame(200, static::$client->getResponse()->getStatusCode());
+        static::assertGreaterThan(
+            0,
+            $crawler->filter('html:contains("DefaultController::attributeFooEnabledBarDisabledAction")')->count(),
+        );
+    }
 }
diff --git a/tests/Unit/ExpressionLanguage/FeatureFlagExpressionLanguageProviderTest.php b/tests/Unit/ExpressionLanguage/FeatureFlagExpressionLanguageProviderTest.php
@@ -28,7 +28,7 @@ protected function setUp(): void
         $this->featureManager = $this->createMock(FeatureManager::class);
 
         $this->expressionLanguage = new class($this->featureManager) extends ExpressionLanguage {
-            public function __construct($featureManager, CacheItemPoolInterface $cache = null, array $providers = [])
+            public function __construct($featureManager, ?CacheItemPoolInterface $cache = null, array $providers = [])
             {
                 array_unshift($providers, new FeatureFlagExpressionLanguageProvider($featureManager));