Account for traffic allocation on all flags (#87)

keelerm84 · web-flow · commit dea0c9292495 · 2022-02-02T16:21:46.000-05:00
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -75,13 +75,12 @@ jobs:
       - run:
           name: php-cs-fixer check
           command: composer cs-check
-      - run: mkdir -p ~/phpunit
       - run:
           name: run tests with highest compatible dependency versions
           command: php -d xdebug.mode=coverage vendor/bin/phpunit
           enviroment:
             XDEBUG_MODE: coverage
       - store_test_results:
-          path: ~/phpunit
+          path: build/phpunit
       - store_artifacts:
-          path: ~/phpunit
+          path: build/phpunit
diff --git a/build/.gitignore b/build/.gitignore
@@ -0,0 +1,2 @@
+*
+!.gitignore
diff --git a/phpunit.xml b/phpunit.xml
@@ -2,16 +2,16 @@
 <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd" bootstrap="vendor/autoload.php" colors="true" beStrictAboutChangesToGlobalState="true" beStrictAboutOutputDuringTests="true" beStrictAboutResourceUsageDuringSmallTests="true" beStrictAboutTestsThatDoNotTestAnything="true" beStrictAboutTodoAnnotatedTests="true" verbose="true" defaultTimeLimit="10">
   
   <logging>
-    <junit outputFile="~/phpunit/junit.xml"/>
+    <junit outputFile="build/phpunit/junit.xml"/>
   </logging>
 
   <coverage>
     <include>
       <directory suffix=".php">src</directory>
     </include>
     <report>
-      <html outputDirectory="~/phpunit/html-coverage"/>
-      <xml outputDirectory="~/phpunit/xml-coverage"/>
+      <html outputDirectory="build/phpunit/html-coverage"/>
+      <xml outputDirectory="build/phpunit/xml-coverage"/>
     </report>
   </coverage>
 
diff --git a/src/LaunchDarkly/FeatureFlagsState.php b/src/LaunchDarkly/FeatureFlagsState.php
@@ -47,20 +47,38 @@ public function addFlag(
         bool $withReason = false,
         bool $detailsOnlyIfTracked = false
     ): void {
+        $requireExperimentData = $flag->isExperiment($detail->getReason());
+
         $this->_flagValues[$flag->getKey()] = $detail->getValue();
         $meta = [];
-        if (!$detailsOnlyIfTracked || $flag->isTrackEvents() || $flag->getDebugEventsUntilDate()) {
-            $meta['version'] = $flag->getVersion();
-            if ($withReason) {
-                $meta['reason'] = $detail->getReason();
+
+        $trackEvents = $flag->isTrackEvents() || $requireExperimentData;
+        $trackReason = $requireExperimentData;
+
+        $omitDetails = false;
+        if ($detailsOnlyIfTracked) {
+            if (!$trackEvents && !$trackReason && !$flag->getDebugEventsUntilDate()) {
+                $omitDetails = true;
             }
         }
+
+        $reason = (!$withReason && !$trackReason) ? null : $detail->getReason();
+
+        if ($reason && !$omitDetails) {
+            $meta['reason'] = $reason;
+        }
+        if (!$omitDetails) {
+            $meta['version'] = $flag->getVersion();
+        }
         if (!is_null($detail->getVariationIndex())) {
             $meta['variation'] = $detail->getVariationIndex();
         }
-        if ($flag->isTrackEvents()) {
+        if ($trackEvents) {
             $meta['trackEvents'] = true;
         }
+        if ($trackReason) {
+            $meta['trackReason'] = true;
+        }
         if ($flag->getDebugEventsUntilDate()) {
             $meta['debugEventsUntilDate'] = $flag->getDebugEventsUntilDate();
         }
diff --git a/src/LaunchDarkly/Impl/Events/EventFactory.php b/src/LaunchDarkly/Impl/Events/EventFactory.php
@@ -3,7 +3,6 @@
 namespace LaunchDarkly\Impl\Events;
 
 use LaunchDarkly\EvaluationDetail;
-use LaunchDarkly\EvaluationReason;
 use LaunchDarkly\Impl\Model\FeatureFlag;
 use LaunchDarkly\Impl\Util;
 use LaunchDarkly\LDUser;
@@ -37,7 +36,7 @@ public function newEvalEvent(
         $default,
         $prereqOfFlag = null
     ): array {
-        $addExperimentData = static::isExperiment($flag, $detail->getReason());
+        $addExperimentData = $flag->isExperiment($detail->getReason());
         $e = [
             'kind' => 'feature',
             'creationDate' => Util::currentTimeUnixMillis(),
@@ -186,21 +185,4 @@ private static function contextKind(LDUser $user): string
             return 'user';
         }
     }
-
-    private static function isExperiment(FeatureFlag $flag, EvaluationReason $reason): bool
-    {
-        if ($reason->isInExperiment()) {
-            return true;
-        }
-        switch ($reason->getKind()) {
-            case 'RULE_MATCH':
-                $i = $reason->getRuleIndex();
-                $rules = $flag->getRules();
-                return isset($i) && $i >= 0 && $i < count($rules) && $rules[$i]->isTrackEvents();
-            case 'FALLTHROUGH':
-                return $flag->isTrackEventsFallthrough();
-            default:
-                return false;
-        }
-    }
 }
diff --git a/src/LaunchDarkly/Impl/Model/FeatureFlag.php b/src/LaunchDarkly/Impl/Model/FeatureFlag.php
@@ -137,6 +137,24 @@ public function evaluate(LDUser $user, FeatureRequester $featureRequester, Event
         return new EvalResult($detail, $prereqEvents);
     }
 
+    public function isExperiment(EvaluationReason $reason): bool
+    {
+        if ($reason->isInExperiment()) {
+            return true;
+        }
+
+        switch ($reason->getKind()) {
+            case 'RULE_MATCH':
+                $i = $reason->getRuleIndex();
+                $rules = $this->getRules();
+                return isset($i) && $i >= 0 && $i < count($rules) && $rules[$i]->isTrackEvents();
+            case 'FALLTHROUGH':
+                return $this->isTrackEventsFallthrough();
+            default:
+                return false;
+        }
+    }
+
     private function evaluateInternal(
         LDUser $user,
         FeatureRequester $featureRequester,
diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php
@@ -341,6 +341,11 @@ public function identify(LDUser $user): void
      * - `clientSideOnly`: Set this to true to specify that only flags marked for client-side use
      * should be included; by default, all flags are included
      * - `withReasons`: Set this to true to include evaluation reasons (see {@see \LaunchDarkly\LDClient::variationDetail()})
+     * - `detailsOnlyForTrackedFlags`: Set to true to omit any metadata that is
+     * normally only used for event generation, such as flag versions and
+     * evaluation reasons, unless the flag has event tracking or debugging
+     * turned on
+     *
      * @return FeatureFlagsState a FeatureFlagsState object (will never be null)
      */
     public function allFlagsState(LDUser $user, array $options = []): FeatureFlagsState
diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php
@@ -377,6 +377,54 @@ public function testAllFlagsStateReturnsState()
         $this->assertEquals($expectedState, $state->jsonSerialize());
     }
 
+    public function testAllFlagsStateHandlesExperimentationReasons()
+    {
+        $flagJson = [
+            'key' => 'feature',
+            'version' => 100,
+            'deleted' => false,
+            'on' => true,
+            'targets' => [],
+            'prerequisites' => [],
+            'rules' => [],
+            'offVariation' => 1,
+            'fallthrough' => ['variation' => 0],
+            'variations' => ['fall', 'off', 'on'],
+            'salt' => '',
+            'trackEvents' => false,
+            'trackEventsFallthrough' => true,
+            'debugEventsUntilDate' => 1000
+        ];
+        $flag = FeatureFlag::decode($flagJson);
+
+        MockFeatureRequester::$flags = ['feature' => $flag];
+        $client = $this->makeClient();
+
+        $builder = new LDUserBuilder(3);
+        $user = $builder->build();
+        $state = $client->allFlagsState($user);
+
+        $this->assertTrue($state->isValid());
+        $this->assertEquals(['feature' => 'fall'], $state->toValuesMap());
+        $expectedState = [
+            'feature' => 'fall',
+            '$flagsState' => [
+                'feature' => [
+                    'variation' => 0,
+                    'version' => 100,
+                    'trackEvents' => true,
+                    'trackReason' => true,
+                    'debugEventsUntilDate' => 1000,
+                    'reason' => [
+                        'kind' => 'FALLTHROUGH',
+                    ],
+                ]
+            ],
+            '$valid' => true
+        ];
+        $this->assertEquals($expectedState, $state->jsonSerialize());
+    }
+
     public function testAllFlagsStateReturnsStateWithReasons()
     {
         $flagJson = [
