feat: Evaluate flags that depend on other flags

.gitignore

@@ -4,3 +4,4 @@ composer.lock
 /node_modules/
 .env
 .php-cs-fixer.cache
+.phpbench/

.gitmodules

@@ -1,4 +1,4 @@
 [submodule "tests/Engine/EngineTests/EngineTestData"]
 	path = tests/Engine/EngineTests/EngineTestData
 	url = git@github.com:Flagsmith/engine-test-data.git
-	branch = v3.7.0
+	branch = v3.11.0

benchmarks/EngineBench.php

@@ -0,0 +1,82 @@
+<?php
+
+namespace FlagsmithBench;
+
+use Flagsmith\Engine\Engine;
+use Flagsmith\Engine\Utils\Types\Context\EvaluationContext;
+use Flagsmith\Flagsmith;
+use Flagsmith\Offline\LocalFileHandler;
+use PhpBench\Attributes as Bench;
+
+#[Bench\BeforeMethods('setUp')]
+#[Bench\Revs(1000)]
+#[Bench\Iterations(10)]
+#[Bench\Warmup(1)]
+#[Bench\OutputTimeUnit('microseconds', precision: 2)]
+#[Bench\RetryThreshold(5.0)]
+class EngineBench
+{
+    private const TEST_CASES_PATH = __DIR__ . '/../tests/Engine/EngineTests/EngineTestData/test_cases';
+
+    /** Same contexts as the flagsmith-engine CodSpeed benchmark. */
+    private const ENGINE_BENCHMARK_TEST_CASES = [
+        'test_0cfd0d72-4de4-4ed7-9cfb-d80dc3dacead__default.json',
+        'test_1bde8445-ca19-4bda-a9d5-3543a800fc0f__context_values.json',
+        'test_in_condition_json_array_format__should_match.jsonc',
+        'test_in_condition_numeric_comma_separated__should_match.jsonc',
+        'test_in_condition_array_matching_value__should_match.jsonc',
+    ];
+
+    /** @var array<EvaluationContext> */
+    private array $engineContexts;
+
+    /** @var array<EvaluationContext> */
+    private array $flagDependencyContexts;
+
+    private Flagsmith $flagsmith;
+
+    public function setUp(): void
+    {
+        $this->engineContexts = array_map(
+            fn (string $fileName) => self::loadContext(self::TEST_CASES_PATH . "/{$fileName}"),
+            self::ENGINE_BENCHMARK_TEST_CASES,
+        );
+        $this->flagDependencyContexts = array_map(
+            fn (string $path) => self::loadContext($path),
+            glob(self::TEST_CASES_PATH . '/test_flag_dependency__*.jsonc'),
+        );
+        $this->flagsmith = new Flagsmith(
+            offlineMode: true,
+            offlineHandler: new LocalFileHandler(__DIR__ . '/../tests/Data/environment.json'),
+        );
+    }
+
+    public function benchEngine(): void
+    {
+        foreach ($this->engineContexts as $context) {
+            Engine::getEvaluationResult($context);
+        }
+    }
+
+    public function benchEngineFlagDependency(): void
+    {
+        foreach ($this->flagDependencyContexts as $context) {
+            Engine::getEvaluationResult($context);
+        }
+    }
+
+    public function benchLocalEvaluationEnvironmentFlags(): void
+    {
+        $this->flagsmith->getEnvironmentFlags();
+    }
+
+    public function benchLocalEvaluationIdentityFlags(): void
+    {
+        $this->flagsmith->getIdentityFlags('identity', (object) ['foo' => 'bar']);
+    }
+
+    private static function loadContext(string $path): EvaluationContext
+    {
+        return EvaluationContext::fromJsonObject(json5_decode(file_get_contents($path))->context);
+    }
+}

composer.json

@@ -27,7 +27,8 @@
         "symfony/cache": "^5.4.6",
         "friendsofphp/php-cs-fixer": "^3.6",
         "doppiogancio/mocked-client": "^3.0",
-        "colinodell/json5": "^3.0"
+        "colinodell/json5": "^3.0",
+        "phpbench/phpbench": "^1.4"
     },
     "autoload": {
         "psr-4": {

phpbench.json

@@ -0,0 +1,6 @@
+{
+    "$schema": "./vendor/phpbench/phpbench/phpbench.schema.json",
+    "runner.bootstrap": "vendor/autoload.php",
+    "runner.path": "benchmarks",
+    "runner.file_pattern": "*Bench.php"
+}

src/Engine/Engine.php

@@ -3,6 +3,7 @@
 namespace Flagsmith\Engine;
 
 use Flagsmith\Engine\Utils\Hashing;
+use Flagsmith\Engine\Utils\LazyFlags;
 use Flagsmith\Engine\Utils\Semver;
 use Flagsmith\Engine\Utils\Types\Context\EvaluationContext;
 use Flagsmith\Engine\Utils\Types\Context\FeatureContext;
@@ -46,8 +47,28 @@ public static function getEvaluationResult($context): EvaluationResult
 
         $context = self::getEnrichedContext($context);
 
+        // Discard any caller-supplied `$.flags`: they are resolved lazily as
+        // segment conditions read them. Without segments, nothing reads them.
+        $resolvedFlags = null;
+        if (!empty($context->segments)) {
+            $context = clone $context;
+            $resolvedFlags = new LazyFlags($context);
+            $context->flags = $resolvedFlags;
+        }
+
+        $cycleHits = 0;
+
         foreach ($context->segments as $segment) {
-            if (!self::isContextInSegment($context, $segment)) {
+            $matches = self::isContextInSegment($context, $segment);
+
+            if ($resolvedFlags->getCycleHits() !== $cycleHits) {
+                // Evaluating this segment cut a cycle, so it matched, or failed to,
+                // on a flag that could not be resolved. Neither verdict is sound.
+                $cycleHits = $resolvedFlags->getCycleHits();
+                continue;
+            }
+
+            if (!$matches) {
                 continue;
             }
 
@@ -58,14 +79,8 @@ public static function getEvaluationResult($context): EvaluationResult
 
             foreach ($segment->overrides as $overrideFeature) {
                 $featureName = $overrideFeature->name;
-                $evaluatedFeature = $evaluatedFeatures[$featureName] ?? null;
-                if ($evaluatedFeature) {
-                    $overrideWinsPriority =
-                        ($overrideFeature->priority ?? self::WEAKEST_PRIORITY) <
-                        ($evaluatedFeature->priority ?? self::WEAKEST_PRIORITY);
-                    if (!$overrideWinsPriority) {
-                        continue;
-                    }
+                if (!self::overrideWins($overrideFeature, $evaluatedFeatures[$featureName] ?? null)) {
+                    continue;
                 }
 
                 $evaluatedFeatures[$featureName] = $overrideFeature;
@@ -89,6 +104,24 @@ public static function getEvaluationResult($context): EvaluationResult
             );
         }
 
+        if ($resolvedFlags !== null) {
+            $context->flags = null; // Frees both by reference counting, not by the cycle collector
+        }
+
+        if ($resolvedFlags?->isUsed()) {
+            // Only reached when a segment condition read a flag. Those results take
+            // precedence: they were resolved with the cycle guard, unlike the
+            // single-pass recomputation above.
+            foreach ($resolvedFlags->getResolved() as $featureName => $flag) {
+                $evaluatedFlags[$featureName] = $flag;
+            }
+            foreach ($resolvedFlags->getCyclic() as $featureName) {
+                if (isset($evaluatedFlags[$featureName])) {
+                    $evaluatedFlags[$featureName]->reason = LazyFlags::CIRCULAR_DEPENDENCY_REASON;
+                }
+            }
+        }
+
         $result = new EvaluationResult();
         $result->flags = $evaluatedFlags;
         $result->segments = $evaluatedSegments;
@@ -114,11 +147,29 @@ private static function getEnrichedContext(EvaluationContext $context): Evaluati
     }
 
     /**
+     * Whether a segment override takes precedence over the one held so far.
+     *
+     * Lower priority wins, and the first seen wins a tie, so that precedence
+     * follows context order rather than the order segments happen to be
+     * evaluated in.
+     * @internal
+     * @param FeatureContext $candidate
+     * @param ?FeatureContext $incumbent
+     * @return bool
+     */
+    public static function overrideWins($candidate, $incumbent): bool
+    {
+        return $incumbent === null
+            || ($candidate->priority ?? self::WEAKEST_PRIORITY) < ($incumbent->priority ?? self::WEAKEST_PRIORITY);
+    }
+
+    /**
+     * @internal
      * @param EvaluationContext $context
      * @param SegmentContext $segment
      * @return bool
      */
-    private static function isContextInSegment($context, $segment): bool
+    public static function isContextInSegment($context, $segment): bool
     {
         if (empty($segment->rules)) {
             return false;
@@ -134,14 +185,20 @@ private static function isContextInSegment($context, $segment): bool
     }
 
     /**
+     * @internal
      * @param FeatureContext $feature
      * @param ?string $splitKey
      * @param string $reason
      * @return FlagResult
      */
-    private static function getFlagResultFromFeatureContext($feature, $splitKey, $reason = 'DEFAULT')
+    public static function getFlagResultFromFeatureContext($feature, $splitKey, $reason = 'DEFAULT')
     {
+        $variantKey = null;
+
         if ($splitKey !== null && !empty($feature->variants)) {
+            // Default to the control bucket; a matched named variant overrides this.
+            $variantKey = 'control';
+
             $hashing = new Hashing();
             $percentageValue = $hashing->getHashedPercentageForObjectIds([
                 $feature->key,
@@ -163,6 +220,7 @@ private static function getFlagResultFromFeatureContext($feature, $splitKey, $re
                     $flag->name = $feature->name;
                     $flag->enabled = $feature->enabled;
                     $flag->value = $variant->value;
+                    $flag->variant = $variant->key;
                     $flag->reason = "SPLIT; weight={$variant->weight}";
                     $flag->metadata = $feature->metadata;
                     return $flag;
@@ -175,6 +233,7 @@ private static function getFlagResultFromFeatureContext($feature, $splitKey, $re
         $flag->name = $feature->name;
         $flag->enabled = $feature->enabled;
         $flag->value = $feature->value;
+        $flag->variant = $variantKey;
         $flag->reason = $reason;
         $flag->metadata = $feature->metadata;
         return $flag;