diff --git a/.github/workflows/sdk-spec.yml b/.github/workflows/sdk-spec.yml index 3115820..165c997 100644 --- a/.github/workflows/sdk-spec.yml +++ b/.github/workflows/sdk-spec.yml @@ -11,7 +11,8 @@ permissions: jobs: sdk-spec: # Keep the ref in `uses:` and `spec-ref` in sync. - uses: Flagsmith/sdk-spec/.github/workflows/run-spec.yml@v0.3.0 + uses: Flagsmith/sdk-spec/.github/workflows/run-spec.yml@v0.7.0 with: - spec-ref: v0.3.0 + spec-ref: v0.7.0 + tags: core or local-evaluation language: php diff --git a/.gitmodules b/.gitmodules index 47f5132..0d1db87 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,7 +1,7 @@ [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.13.2 [submodule "tests/Engine/EngineTests/SdkBenchmarks"] path = tests/Engine/EngineTests/SdkBenchmarks url = https://github.com/Flagsmith/sdk-benchmarks.git diff --git a/sdk-spec/steps.php b/sdk-spec/steps.php index 9b2812b..deb1464 100644 --- a/sdk-spec/steps.php +++ b/sdk-spec/steps.php @@ -25,6 +25,14 @@ function createClient(string $environmentKey, string $apiUrl): Flagsmith return new Flagsmith(apiKey: $environmentKey, host: $apiUrl); } +// Create the SDK client with local evaluation +function createClientWithLocalEvaluation(string $environmentKey, string $apiUrl): Flagsmith +{ + $flagsmith = new Flagsmith(apiKey: $environmentKey, host: $apiUrl, environmentTtl: 60); + $flagsmith->updateEnvironment(); + return $flagsmith; +} + // When the application gets the environment flags function getEnvironmentFlags(Flagsmith $flagsmith): array { diff --git a/src/Engine/Engine.php b/src/Engine/Engine.php index f092b78..a6387e7 100644 --- a/src/Engine/Engine.php +++ b/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,35 @@ 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; + } + + /** @var array Names of the features overridden by any segment */ + $targetedFeatureNames = []; + + $cycleHits = 0; + foreach ($context->segments as $segment) { - if (!self::isContextInSegment($context, $segment)) { + foreach ($segment->overrides as $overrideFeature) { + $targetedFeatureNames[$overrideFeature->name] = true; + } + + $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 +86,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; @@ -76,7 +98,7 @@ public static function getEvaluationResult($context): EvaluationResult foreach ($context->features as $feature) { $featureName = $feature->name; $evaluatedFeature = $evaluatedFeatures[$featureName] ?? null; - $reason = 'DEFAULT'; + $reason = self::getDefaultReason($featureName, $targetedFeatureNames); if ($evaluatedFeature) { $feature = $evaluatedFeature; $reason = "TARGETING_MATCH; segment={$matchedSegmentsByFeatureName[$featureName]->name}"; @@ -89,6 +111,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 +154,44 @@ 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); + } + + /** + * The reason for a flag served by its environment default. + * + * DEFAULT if any segment overrides the feature, though none matched, or + * STATIC if no segment overrides it. + * @internal + * @param string $featureName + * @param array $targetedFeatureNames Keyed by the names of the features any segment overrides + * @return string + */ + public static function getDefaultReason($featureName, $targetedFeatureNames): string + { + return isset($targetedFeatureNames[$featureName]) ? 'DEFAULT' : 'STATIC'; + } + + /** + * @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 +207,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 +242,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 +255,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; @@ -377,13 +458,18 @@ private static function _contextMatchesCondition( return version_compare($contextValue, $actualVersion, $operator); } + $conditionValue = $cast($condition->value); + if ($conditionValue === null) { + return false; // Not coercible to the context value type + } + return match ($operator) { - '==' => $contextValue === $cast($condition->value), - '!=' => $contextValue !== $cast($condition->value), - '>' => $contextValue > $cast($condition->value), - '>=' => $contextValue >= $cast($condition->value), - '<' => $contextValue < $cast($condition->value), - '<=' => $contextValue <= $cast($condition->value), + '==' => $contextValue === $conditionValue, + '!=' => $contextValue !== $conditionValue, + '>' => $contextValue > $conditionValue, + '>=' => $contextValue >= $conditionValue, + '<' => $contextValue < $conditionValue, + '<=' => $contextValue <= $conditionValue, }; } @@ -403,9 +489,15 @@ private static function _getContextValue($context, $property) } if (str_starts_with($property, '$.')) { + $memberNames = self::_getMemberNames($property); + if ($memberNames !== null) { + $value = self::_digMemberNames($context, $memberNames); + return in_array(gettype($value), self::VALID_CONTEXT_VALUE_TYPES) ? $value : null; + } + try { $jsonpath = new JSONPath($context); - $results = $jsonpath->find($property)->getData(); + $results = $jsonpath->find(self::_decodeUnicodeEscapes($property))->getData(); } catch (JSONPathException) { return null; } @@ -422,6 +514,55 @@ private static function _getContextValue($context, $property) return null; } + // Decode runs of `\uXXXX` escapes, keeping escaped backslashes as written + private static function _decodeUnicodeEscapes(string $property): string + { + return preg_replace_callback( + '/\\\\\\\\|(?:\\\\u[0-9a-fA-F]{4})+/', + fn ($match) => $match[0] === '\\\\' ? $match[0] : json_decode("\"{$match[0]}\"") ?? $match[0], + $property, + ); + } + + /** + * The member names of a JSONPath made of member names only, e.g. `$.identity.identifier`, or null for any other path + * @param string $property + * @return ?array + */ + private static function _getMemberNames($property): ?array + { + static $memberNamesByProperty = []; + if (!array_key_exists($property, $memberNamesByProperty)) { + $memberNamesByProperty[$property] = preg_match('/^\$(\.[\p{L}\p{N}_-]+)+$/u', $property) + ? explode('.', substr($property, 2)) + : null; + } + return $memberNamesByProperty[$property]; + } + + /** + * Read a context value by member names, as a JSONPath of member names only would + * @param EvaluationContext $context + * @param array $memberNames + * @return ?mixed + */ + private static function _digMemberNames($context, $memberNames) + { + $value = $context; + foreach ($memberNames as $memberName) { + $value = match (true) { + is_array($value) => $value[$memberName] ?? null, + $value instanceof \ArrayAccess => $value[$memberName], + is_object($value) => $value->$memberName ?? null, + default => null, + }; + if ($value === null) { + return null; + } + } + return $value; + } + /** * Get a condition value type caster according to a context value * @param mixed $contextValue @@ -436,8 +577,8 @@ private static function _getCaster($contextValue): ?callable return match (gettype($contextValue)) { 'boolean' => fn ($v) => !in_array($v, ['False', 'false']), 'string' => 'strval', - 'integer' => fn ($v) => is_numeric($v) ? (int) $v : $v, - 'double' => fn ($v) => is_numeric($v) ? (float) $v : $v, + 'integer' => fn ($v) => filter_var($v, FILTER_VALIDATE_INT, FILTER_NULL_ON_FAILURE), + 'double' => fn ($v) => filter_var($v, FILTER_VALIDATE_FLOAT, FILTER_NULL_ON_FAILURE), }; } } diff --git a/src/Engine/Utils/LazyFlags.php b/src/Engine/Utils/LazyFlags.php new file mode 100644 index 0000000..e0af20c --- /dev/null +++ b/src/Engine/Utils/LazyFlags.php @@ -0,0 +1,215 @@ + + */ +final class LazyFlags implements \ArrayAccess +{ + public const CIRCULAR_DEPENDENCY_REASON = 'ERROR; code=CIRCULAR_DEPENDENCY'; + + /** @var array */ + private array $flags = []; + + /** + * Names of the flags found to be in a dependency cycle. + * @var array + */ + private array $cyclic = []; + + /** + * Set the first time a condition reads a flag. Distinguishes "no + * dependency was ever consulted" from "one was, and resolved to nothing", + * which an empty mapping cannot. + */ + private bool $used = false; + + /** + * Incremented whenever a cycle is cut. A segment whose own evaluation + * increments it read a flag that could not be resolved, so its verdict + * rests on that flag's absence and it must not match. + */ + private int $cycleHits = 0; + + /** + * Stack of the features currently being resolved. + * @var array + */ + private array $resolving = []; + + /** @var array */ + private array $segmentMatches = []; + + /** @var ?array> */ + private ?array $segmentKeysByFeatureName = null; + + public function __construct(private EvaluationContext $context) + { + } + + public function isUsed(): bool + { + return $this->used; + } + + public function getCycleHits(): int + { + return $this->cycleHits; + } + + /** @return array */ + public function getResolved(): array + { + return $this->flags; + } + + /** @return array */ + public function getCyclic(): array + { + return array_keys($this->cyclic); + } + + public function offsetExists(mixed $offset): bool + { + return $this->offsetGet($offset) !== null; + } + + public function offsetGet(mixed $offset): ?FlagResult + { + $featureName = (string) $offset; + if (!array_key_exists($featureName, $this->flags)) { + $this->used = true; + $this->resolveFeature($featureName); + } + return $this->flags[$featureName] ?? null; + } + + public function offsetSet(mixed $offset, mixed $value): void + { + throw new \LogicException('$.flags is read-only'); + } + + public function offsetUnset(mixed $offset): void + { + throw new \LogicException('$.flags is read-only'); + } + + /** + * Resolve the flag for a feature, applying any segment override. + * @param string $featureName + * @return void + */ + private function resolveFeature(string $featureName): void + { + $cycleStart = array_search($featureName, $this->resolving, strict: true); + if ($cycleStart !== false) { + // Cyclic dependency. Leave the flag unresolved. + foreach (array_slice($this->resolving, $cycleStart) as $cyclicFeatureName) { + $this->cyclic[$cyclicFeatureName] = true; + } + $this->cycleHits++; + return; + } + + $feature = $this->context->features[$featureName] ?? null; + if ($feature === null) { + // Depending on a feature absent from the context. + return; + } + + $this->resolving[] = $featureName; + try { + [$overrideFeature, $overrideSegment] = $this->getSegmentOverride($featureName); + } finally { + array_pop($this->resolving); + } + + if (isset($this->cyclic[$featureName])) { + // The result is not something another condition may match on. + return; + } + + $this->flags[$featureName] = $overrideFeature !== null + ? Engine::getFlagResultFromFeatureContext( + $overrideFeature, + $this->context->identity?->key, + "TARGETING_MATCH; segment={$overrideSegment->name}", + ) + : Engine::getFlagResultFromFeatureContext( + $feature, + $this->context->identity?->key, + Engine::getDefaultReason($featureName, $this->segmentKeysByFeatureName), + ); + } + + /** + * @param string $segmentKey + * @return bool + */ + private function matchesSegment(string $segmentKey): bool + { + if (array_key_exists($segmentKey, $this->segmentMatches)) { + return $this->segmentMatches[$segmentKey]; + } + + $cycleHits = $this->cycleHits; + $matches = Engine::isContextInSegment($this->context, $this->context->segments[$segmentKey]); + + if ($this->cycleHits !== $cycleHits) { + // Reached by cutting a cycle, so the verdict rests on a flag that + // could not be resolved. Not a match, and not worth memoising. + return false; + } + + $this->segmentMatches[$segmentKey] = $matches; + return $matches; + } + + /** + * @param string $featureName + * @return array{?FeatureContext, ?SegmentContext} The winning override and its segment + */ + private function getSegmentOverride(string $featureName): array + { + if ($this->segmentKeysByFeatureName === null) { + $this->segmentKeysByFeatureName = []; + foreach ($this->context->segments as $segmentKey => $segment) { + foreach ($segment->overrides as $override) { + $this->segmentKeysByFeatureName[$override->name][] = $segmentKey; + } + } + } + + $overrideFeature = null; + $overrideSegment = null; + foreach ($this->segmentKeysByFeatureName[$featureName] ?? [] as $segmentKey) { + if (!$this->matchesSegment($segmentKey)) { + continue; + } + $segment = $this->context->segments[$segmentKey]; + foreach ($segment->overrides as $override) { + if ($override->name !== $featureName) { + continue; + } + if (Engine::overrideWins($override, $overrideFeature)) { + $overrideFeature = $override; + $overrideSegment = $segment; + } + } + } + + return [$overrideFeature, $overrideSegment]; + } +} diff --git a/src/Engine/Utils/Types/Context/EvaluationContext.php b/src/Engine/Utils/Types/Context/EvaluationContext.php index 9636642..e0b13ea 100644 --- a/src/Engine/Utils/Types/Context/EvaluationContext.php +++ b/src/Engine/Utils/Types/Context/EvaluationContext.php @@ -2,6 +2,8 @@ namespace Flagsmith\Engine\Utils\Types\Context; +use Flagsmith\Engine\Utils\Types\Result\FlagResult; + class EvaluationContext { /** @var EnvironmentContext */ @@ -16,6 +18,13 @@ class EvaluationContext /** @var array */ public $features; + /** + * Read-only: populated by the engine as flags are resolved. Any value + * supplied by the caller is discarded at evaluation time. + * @var null|array|\ArrayAccess + */ + public $flags; + /** * @param object $jsonContext * @return EvaluationContext @@ -112,6 +121,7 @@ private static function _convertFeatures($jsonFeatures): array $feature->variants = []; foreach ($jsonFeature->variants ?? [] as $jsonVariant) { $variant = new FeatureValue(); + $variant->key = $jsonVariant->key ?? null; $variant->value = $jsonVariant->value; $variant->weight = $jsonVariant->weight; $variant->priority = $jsonVariant->priority; diff --git a/src/Engine/Utils/Types/Context/FeatureValue.php b/src/Engine/Utils/Types/Context/FeatureValue.php index 077068d..df39239 100644 --- a/src/Engine/Utils/Types/Context/FeatureValue.php +++ b/src/Engine/Utils/Types/Context/FeatureValue.php @@ -4,6 +4,9 @@ class FeatureValue { + /** @var ?string */ + public $key; + /** @var mixed */ public $value; diff --git a/src/Engine/Utils/Types/Result/FlagResult.php b/src/Engine/Utils/Types/Result/FlagResult.php index f68742b..59c3168 100644 --- a/src/Engine/Utils/Types/Result/FlagResult.php +++ b/src/Engine/Utils/Types/Result/FlagResult.php @@ -13,6 +13,9 @@ class FlagResult implements \JsonSerializable /** @var ?mixed */ public $value; + /** @var ?string */ + public $variant; + /** @var ?string */ public $reason; diff --git a/src/Flagsmith.php b/src/Flagsmith.php index 3458cdd..dab04e7 100644 --- a/src/Flagsmith.php +++ b/src/Flagsmith.php @@ -50,6 +50,7 @@ class Flagsmith private bool $useCacheAsFailover = false; private array $headers = []; private ?EvaluationContext $localEvaluationContext = null; + private ?EvaluationContext $environmentFlagsContext = null; private bool $offlineMode = false; private ?IOfflineHandler $offlineHandler = null; @@ -81,7 +82,7 @@ public function __construct( // The offline handler can be used as backup if online but the request fails if (!is_null($offlineHandler)) { $environment = $offlineHandler->getEnvironment(); - $this->localEvaluationContext = Mappers::mapEnvironmentDocumentToContext($environment); + $this->setLocalEvaluationContext(Mappers::mapEnvironmentDocumentToContext($environment)); } if (!$offlineMode) { @@ -400,8 +401,20 @@ public function updateEnvironment(): void ttl: $this->environmentTtl, ); - $context = Mappers::mapEnvironmentDocumentToContext($environmentData); + $this->setLocalEvaluationContext(Mappers::mapEnvironmentDocumentToContext($environmentData)); + } + + /** + * Set the context for local evaluation, and the one environment flags are evaluated against. + * @param EvaluationContext $context + * @return void + */ + private function setLocalEvaluationContext(EvaluationContext $context): void + { $this->localEvaluationContext = $context; + + $this->environmentFlagsContext = clone $context; + $this->environmentFlagsContext->segments = Mappers::selectIdentityFreeSegments($context->segments); } /** @@ -410,15 +423,11 @@ public function updateEnvironment(): void */ private function getEnvironmentFlagsFromLocalEvaluationContext(): Flags { - if ($this->localEvaluationContext === null) { + if ($this->environmentFlagsContext === null) { throw new FlagsmithClientError('Evaluation context is not present'); } - // Omit segments from evaluation context for environment flags - $context = $this->localEvaluationContext->deepClone(); - $context->segments = []; - - $evaluationResult = Engine::getEvaluationResult($context); + $evaluationResult = Engine::getEvaluationResult($this->environmentFlagsContext); return Flags::fromEvaluationResult( $evaluationResult, diff --git a/src/Utils/Mappers.php b/src/Utils/Mappers.php index 9aa1913..5697548 100644 --- a/src/Utils/Mappers.php +++ b/src/Utils/Mappers.php @@ -16,6 +16,9 @@ class Mappers { + /** Condition properties resolvable without an identity */ + private const IDENTITY_FREE_PROPERTY_PREFIXES = ['$.environment.', '$.environment[', '$.flags.', '$.flags[']; + /** * Parse the environment document into an EvaluationContext object * @param object $environmentDocument @@ -66,11 +69,54 @@ public static function mapContextAndIdentityToContext($context, $identifier, $tr $identity->identifier = $identifier; $identity->traits = (array) $traits; - $context = $context->deepClone(); + $context = clone $context; // The engine does not modify the context it evaluates $context->identity = $identity; return $context; } + /** + * Select the segments that can be evaluated without an identity, i.e. reading neither identity nor traits + * @param array $segments + * @return array + */ + public static function selectIdentityFreeSegments($segments): array + { + return array_filter($segments, fn ($segment) => self::_isIdentityFree($segment->rules)); + } + + /** + * @param ?array $rules + * @return bool + */ + private static function _isIdentityFree($rules): bool + { + foreach ($rules ?? [] as $rule) { + foreach ($rule->conditions ?? [] as $condition) { + if (!self::_isIdentityFreeProperty($condition->property)) { + return false; + } + } + if (!self::_isIdentityFree($rule->rules)) { + return false; + } + } + return true; + } + + /** + * @param ?string $property + * @return bool + */ + private static function _isIdentityFreeProperty($property): bool + { + foreach (self::IDENTITY_FREE_PROPERTY_PREFIXES as $prefix) { + if (str_starts_with((string) $property, $prefix)) { + return true; + } + } + return false; + } + /** * @param array $srcRules * @return array @@ -184,6 +230,7 @@ private static function _mapIdentityOverridesToSegments($identityOverrides) $identifiersRule = new SegmentRule(); $identifiersRule->type = SegmentRuleType::ALL; $identifiersRule->conditions = [$identifiersCondition]; + $identifiersRule->rules = []; $segment->rules = [$identifiersRule]; $segment->overrides = []; diff --git a/tests/Data/environment_with_segment_override.json b/tests/Data/environment_with_segment_override.json index eb56225..c4e7bf9 100644 --- a/tests/Data/environment_with_segment_override.json +++ b/tests/Data/environment_with_segment_override.json @@ -46,6 +46,40 @@ ] } ] + }, + { + "id": 2, + "name": "Premium users", + "feature_states": [ + { + "feature_state_value": "premium-override-value", + "django_id": 20, + "featurestate_uuid": "88888888-8888-8888-8888-888888888888", + "feature": { + "name": "premium_feature", + "type": "STANDARD", + "id": 20 + }, + "enabled": true, + "multivariate_feature_state_values": [], + "feature_segment": { + "priority": 2 + } + } + ], + "rules": [ + { + "type": "ALL", + "rules": [], + "conditions": [ + { + "operator": "EQUAL", + "property_": "plan", + "value": "premium" + } + ] + } + ] } ] }, @@ -64,6 +98,19 @@ "segment_id": null, "enabled": true, "multivariate_feature_state_values": [] + }, + { + "feature_state_value": "premium-default-value", + "django_id": 2, + "featurestate_uuid": "11111111-1111-1111-1111-111111111111", + "feature": { + "name": "premium_feature", + "type": "STANDARD", + "id": 20 + }, + "segment_id": null, + "enabled": false, + "multivariate_feature_state_values": [] } ], "identity_overrides": [] diff --git a/tests/Engine/EngineTests/EngineTestData b/tests/Engine/EngineTests/EngineTestData index 4b29dc7..da82e78 160000 --- a/tests/Engine/EngineTests/EngineTestData +++ b/tests/Engine/EngineTests/EngineTestData @@ -1 +1 @@ -Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f +Subproject commit da82e780a5a662ded227fd1f6f63307166b4cebe diff --git a/tests/FlagsmithClientTest.php b/tests/FlagsmithClientTest.php index dfa4893..cdd334a 100644 --- a/tests/FlagsmithClientTest.php +++ b/tests/FlagsmithClientTest.php @@ -479,7 +479,7 @@ public function testApiRequestsIncludeUserAgentHeader() $this->assertEquals($expectedUserAgent, $userAgent); } - public function testGetEnvironmentFlagsOmitsSegmentsFromEvaluation() + public function testGetEnvironmentFlagsAppliesOnlySegmentsNotNeedingAnIdentity() { // Given $flagsmith = (new Flagsmith('ser.api_key', environmentTtl: 1)) @@ -488,15 +488,19 @@ public function testGetEnvironmentFlagsOmitsSegmentsFromEvaluation() // When $environmentFlags = $flagsmith->getEnvironmentFlags(); - $identityFlags = $flagsmith->getIdentityFlags('test-identity'); + $identityFlags = $flagsmith->getIdentityFlags('test-identity', (object) ['plan' => 'premium']); // Then $environmentFlag = $environmentFlags->getFlag('test_feature'); - $this->assertTrue($environmentFlag->enabled); - $this->assertEquals('environment-default-value', $environmentFlag->value); + $this->assertFalse($environmentFlag->enabled); + $this->assertEquals('segment-override-value', $environmentFlag->value); - $identityFlag = $identityFlags->getFlag('test_feature'); - $this->assertFalse($identityFlag->enabled); - $this->assertEquals('segment-override-value', $identityFlag->value); + $environmentPremiumFlag = $environmentFlags->getFlag('premium_feature'); + $this->assertFalse($environmentPremiumFlag->enabled); + $this->assertEquals('premium-default-value', $environmentPremiumFlag->value); + + $identityPremiumFlag = $identityFlags->getFlag('premium_feature'); + $this->assertTrue($identityPremiumFlag->enabled); + $this->assertEquals('premium-override-value', $identityPremiumFlag->value); } } diff --git a/tests/Utils/MappersTest.php b/tests/Utils/MappersTest.php index a9ebed8..9f75696 100644 --- a/tests/Utils/MappersTest.php +++ b/tests/Utils/MappersTest.php @@ -56,7 +56,7 @@ public function testMapEnvironmentDocumentToContextProducesEvaluationContext(): $this->assertEquals(SegmentRuleType::ALL, $context->segments[$overrideKey]->rules[0]->type); $this->assertCount(1, $context->segments[$overrideKey]->rules[0]->conditions); - $this->assertNull($context->segments[$overrideKey]->rules[0]->rules); + $this->assertEmpty($context->segments[$overrideKey]->rules[0]->rules); $this->assertEquals('$.identity.identifier', $context->segments[$overrideKey]->rules[0]->conditions[0]->property); $this->assertEquals(SegmentConditionOperator::IN, $context->segments[$overrideKey]->rules[0]->conditions[0]->operator); @@ -170,6 +170,41 @@ public function testMapEnvironmentDocumentToContextMapsIdentityOverridesWithMult $this->assertEquals(['id' => 2], $segment->overrides[1]->metadata); } + public function testSelectIdentityFreeSegmentsKeepsOnlySegmentsEvaluableWithoutAnIdentity(): void + { + // Given + $context = EvaluationContext::fromJsonObject(json_decode(<<<'JSON' + { + "environment": {"key": "key", "name": "Production"}, + "segments": { + "on_environment": {"key": "on_environment", "name": "on_environment", "rules": [ + {"type": "ALL", "conditions": [{"property": "$.environment.name", "operator": "EQUAL", "value": "Production"}]} + ]}, + "on_flag": {"key": "on_flag", "name": "on_flag", "rules": [ + {"type": "ANY", "conditions": [{"property": "$.flags['payments'].enabled", "operator": "NOT_EQUAL", "value": "true"}]} + ]}, + "on_nested_trait": {"key": "on_nested_trait", "name": "on_nested_trait", "rules": [ + {"type": "ALL", "conditions": [], "rules": [ + {"type": "ANY", "conditions": [{"property": "plan", "operator": "IS_NOT_SET", "value": null}]} + ]} + ]}, + "on_identifier": {"key": "on_identifier", "name": "on_identifier", "rules": [ + {"type": "ALL", "conditions": [{"property": "$.identity.identifier", "operator": "IN", "value": ["user-1"]}]} + ]}, + "on_identity_split": {"key": "on_identity_split", "name": "on_identity_split", "rules": [ + {"type": "ALL", "conditions": [{"property": null, "operator": "PERCENTAGE_SPLIT", "value": "50"}]} + ]} + } + } + JSON)); + + // When + $identityFreeSegments = Mappers::selectIdentityFreeSegments($context->segments); + + // Then + $this->assertEquals(['on_environment', 'on_flag'], array_keys($identityFreeSegments)); + } + private static function _identityFeatureState(int $featureId, string $featureName, bool $enabled, string $value): object { return (object) [