refactor(Engine): nits from context work review (#117)

* Pop feature ID out of the override key

* Replace flagsmith_id with id

Author: emyller

src/Flagsmith.php

@@ -371,12 +371,12 @@ traits: $traits,
 
         $segments = [];
         foreach ($evaluationResult->segments as $resultSegment) {
-            if (($resultSegment->metadata['flagsmith_id'] ?? null) === null) {
+            if (($resultSegment->metadata['id'] ?? null) === null) {
                 continue;  // Not a real segment, e.g. an identity override virtual segment
             }
 
             $segment = new Segment();
-            $segment->id = $resultSegment->metadata['flagsmith_id'];
+            $segment->id = $resultSegment->metadata['id'];
             $segment->name = $resultSegment->name;
             $segments[] = $segment;
         }

src/Models/Flags.php

@@ -96,7 +96,7 @@ public static function fromEvaluationResult(
         foreach ($evaluationResult->flags as $flagResult) {
             $flag = new Flag();
             $flag->feature_name = $flagResult->name;
-            $flag->feature_id = $flagResult->metadata['flagsmith_id'];
+            $flag->feature_id = $flagResult->metadata['id'];
             $flag->enabled = $flagResult->enabled;
             $flag->value = $flagResult->value;
             $flags[$flagResult->name] = $flag;

src/Utils/Mappers.php

@@ -37,7 +37,7 @@ public static function mapEnvironmentDocumentToContext($environmentDocument): Ev
             $segment->rules = self::_mapEnvironmentDocumentRulesToContextRules($srcSegment->rules ?? []);
             $segment->metadata = [
                 'source' => 'api',
-                'flagsmith_id' => $srcSegment->id,
+                'id' => $srcSegment->id,
             ];
             $context->segments[$segment->key] = $segment;
 
@@ -115,7 +115,7 @@ private static function _mapEnvironmentDocumentFeatureStatesToFeatureContexts($f
             $feature->enabled = $featureState->enabled;
             $feature->value = $featureState->feature_state_value;
             $feature->priority = $featureState->feature_segment?->priority ?? null;
-            $feature->metadata = ['flagsmith_id' => $featureState->feature->id];
+            $feature->metadata = ['id' => $featureState->feature->id];
             $feature->variants = [];
             $multivariateFeatureStateValues = ((array) $featureState->multivariate_feature_state_values) ?? [];
             $multivariateFeatureStateValueUUIDs = array_column($multivariateFeatureStateValues, 'mv_fs_value_uuid');
@@ -141,6 +141,9 @@ private static function _mapEnvironmentDocumentFeatureStatesToFeatureContexts($f
      */
     private static function _mapIdentityOverridesToSegments($identityOverrides)
     {
+        /** @var array<string, int> */
+        $featureIDsByName = [];
+
         /** @var array<string, array<string>> */
         $featuresToIdentifiers = [];
         foreach ($identityOverrides as $identityOverride) {
@@ -154,8 +157,8 @@ private static function _mapIdentityOverridesToSegments($identityOverrides)
             /** @var array<array<mixed>> */
             $overridesKey = [];
             foreach ($identityFeatures as $featureState) {
+                $featureIDsByName[$featureState->feature->name] = $featureState->feature->id;
                 $part = [
-                    $featureState->feature->id,
                     $featureState->feature->name,
                     $featureState->enabled,
                     $featureState->feature_state_value,
@@ -185,14 +188,15 @@ private static function _mapIdentityOverridesToSegments($identityOverrides)
 
             $segment->overrides = [];
             foreach (unserialize($serializedOverridesKey) as $overrideKey) {
-                [$featureId, $featureName, $enabled, $value] = $overrideKey;
+                [$featureName, $enabled, $value] = $overrideKey;
+                $featureId = $featureIDsByName[$featureName];
                 $feature = new FeatureContext();
                 $feature->key = '';  // Not used in identity overrides
                 $feature->name = $featureName;
                 $feature->enabled = $enabled;
                 $feature->value = $value;
                 $feature->priority = Engine::STRONGEST_PRIORITY;
-                $feature->metadata = ['flagsmith_id' => $featureId];
+                $feature->metadata = ['id' => $featureId];
                 $segment->overrides[] = $feature;
             }
 

tests/Utils/MappersTest.php

@@ -33,7 +33,7 @@ public function testMapEnvironmentDocumentToContextProducesEvaluationContext():
         $this->assertCount(1, $context->segments[0]->rules);
         $this->assertEmpty($context->segments[0]->overrides);
         $this->assertEquals('api', $context->segments[0]->metadata['source']);
-        $this->assertEquals('1', $context->segments[0]->metadata['flagsmith_id']);
+        $this->assertEquals('1', $context->segments[0]->metadata['id']);
 
         $this->assertEquals(SegmentRuleType::ALL, $context->segments[0]->rules[0]->type);
         $this->assertEmpty($context->segments[0]->rules[0]->conditions);
@@ -47,7 +47,7 @@ public function testMapEnvironmentDocumentToContextProducesEvaluationContext():
         $this->assertEquals(SegmentConditionOperator::EQUAL, $context->segments[0]->rules[0]->rules[0]->conditions[0]->operator);
         $this->assertEquals('bar', $context->segments[0]->rules[0]->rules[0]->conditions[0]->value);
 
-        $overrideKey = '1dfdec3e4c67121138b1faa01b82f9f731c692842b865f263824bfabf46d5fff';
+        $overrideKey = '2a3691c8a306223592e2e657e50c44cf126db84730e813adea6f951c502b19e8';
         $this->assertArrayHasKey($overrideKey, $context->segments);
         $this->assertEquals('', $context->segments[$overrideKey]->key);
         $this->assertEquals('identity_overrides', $context->segments[$overrideKey]->name);
@@ -68,7 +68,7 @@ public function testMapEnvironmentDocumentToContextProducesEvaluationContext():
         $this->assertEquals('some-overridden-value', $context->segments[$overrideKey]->overrides[0]->value);
         $this->assertEquals(-INF, $context->segments[$overrideKey]->overrides[0]->priority);
         $this->assertNull($context->segments[$overrideKey]->overrides[0]->variants);
-        $this->assertEquals(['flagsmith_id' => 1], $context->segments[$overrideKey]->overrides[0]->metadata);
+        $this->assertEquals(['id' => 1], $context->segments[$overrideKey]->overrides[0]->metadata);
 
         $this->assertCount(3, $context->features);
         $this->assertArrayHasKey('some_feature', $context->features);
@@ -78,7 +78,7 @@ public function testMapEnvironmentDocumentToContextProducesEvaluationContext():
         $this->assertEquals('some-value', $context->features['some_feature']->value);
         $this->assertNull($context->features['some_feature']->priority);
         $this->assertEmpty($context->features['some_feature']->variants);
-        $this->assertEquals(['flagsmith_id' => 1], $context->features['some_feature']->metadata);
+        $this->assertEquals(['id' => 1], $context->features['some_feature']->metadata);
 
         // Test multivariate feature with IDs - priority should be based on ID
         $this->assertArrayHasKey('mv_feature_with_ids', $context->features);
@@ -89,7 +89,7 @@ public function testMapEnvironmentDocumentToContextProducesEvaluationContext():
         $this->assertEquals('default_value', $mvFeatureWithIds->value);
         $this->assertNull($mvFeatureWithIds->priority);
         $this->assertCount(2, $mvFeatureWithIds->variants);
-        $this->assertEquals(['flagsmith_id' => 2], $mvFeatureWithIds->metadata);
+        $this->assertEquals(['id' => 2], $mvFeatureWithIds->metadata);
 
         // First variant: ID=100, should have priority 100
         $this->assertEquals('variant_a', $mvFeatureWithIds->variants[0]->value);
@@ -110,7 +110,7 @@ public function testMapEnvironmentDocumentToContextProducesEvaluationContext():
         $this->assertEquals('fallback_value', $mvFeatureWithoutIds->value);
         $this->assertNull($mvFeatureWithoutIds->priority);
         $this->assertCount(3, $mvFeatureWithoutIds->variants);
-        $this->assertEquals(['flagsmith_id' => 3], $mvFeatureWithoutIds->metadata);
+        $this->assertEquals(['id' => 3], $mvFeatureWithoutIds->metadata);
 
         // Variants should be ordered by UUID alphabetically
         $this->assertEquals('option_y', $mvFeatureWithoutIds->variants[0]->value);