diff --git a/src/EvaluationCore/EvaluationEngine.php b/src/EvaluationCore/EvaluationEngine.php index f76d819..a863235 100644 --- a/src/EvaluationCore/EvaluationEngine.php +++ b/src/EvaluationCore/EvaluationEngine.php @@ -6,7 +6,6 @@ use AmplitudeExperiment\EvaluationCore\Types\EvaluationVariant; use AmplitudeExperiment\EvaluationCore\Types\EvaluationSegment; use AmplitudeExperiment\EvaluationCore\Types\EvaluationCondition; -use Exception; class EvaluationEngine { @@ -117,29 +116,30 @@ private function matchCondition(array $target, EvaluationCondition $condition): if ($propValue === null) { return $this->matchNull($condition->op, $condition->values); - } elseif (is_bool($propValue)) { + } + + if (is_bool($propValue)) { return $this->matchBoolean($propValue, $condition->op, $condition->values); - } elseif ($this->isSetOperator($condition->op)) { - $propValueStringList = $this->coerceStringArray($propValue); + } + $propValueStringList = $this->coerceStringArray($propValue); + + if ($this->isSetOperator($condition->op)) { if ($propValueStringList === null) { return false; } - return $this->matchSet($propValueStringList, $condition->op, $condition->values); - } else { - $propValueString = $this->coerceString($propValue); + } - if ($propValueString !== null) { - return $this->matchString( - $propValueString, - $condition->op, - $condition->values - ); - } else { - return false; - } + if ($propValueStringList !== null) { + return $this->matchStringsNonSet($propValueStringList, $condition->op, $condition->values); } + + $propValueString = $this->coerceString($propValue); + if ($propValueString === null) { + return false; + } + return $this->matchString($propValueString, $condition->op, $condition->values); } /** @@ -300,6 +300,22 @@ private function matchString(string $propValue, string $op, array $filterValues) } } + /** + * @param array $propValues + * @param string $op + * @param array $filterValues + * @return bool + */ + private function matchStringsNonSet(array $propValues, string $op, array $filterValues): bool + { + foreach ($propValues as $propValue) { + if ($this->matchString($propValue, $op, $filterValues)) { + return true; + } + } + return false; + } + /** * @param string $propValue * @param array $filterValues @@ -435,19 +451,28 @@ private function coerceString($value): ?string private function coerceStringArray($value): ?array { if (is_array($value)) { - return array_filter(array_map([$this, 'coerceString'], $value)); + return $this->filterNonNullStrings(array_map([$this, 'coerceString'], $value)); } $stringValue = strval($value); - try { - $parsedValue = json_decode($stringValue, true); - if (is_array($parsedValue)) { - return array_filter(array_map([$this, 'coerceString'], $parsedValue)); - } else { - return null; - } - } catch (Exception $e) { + if (strncmp($stringValue, '[', 1) !== 0) { return null; } + $parsedValue = json_decode($stringValue, true); + if (!is_array($parsedValue)) { + return null; + } + return $this->filterNonNullStrings(array_map([$this, 'coerceString'], $parsedValue)); + } + + /** + * @param array $values + * @return array + */ + private function filterNonNullStrings(array $values): array + { + return array_values(array_filter($values, function ($v) { + return $v !== null; + })); } private function isSetOperator(string $op): bool diff --git a/tests/EvaluationCore/EvaluateIntegrationTest.php b/tests/EvaluationCore/EvaluateIntegrationTest.php index fe14d31..f71a997 100644 --- a/tests/EvaluationCore/EvaluateIntegrationTest.php +++ b/tests/EvaluationCore/EvaluateIntegrationTest.php @@ -26,7 +26,7 @@ class EvaluateIntegrationTest extends TestCase protected function setUp(): void { $this->engine = new EvaluationEngine(); - $rawFlags = $this->getFlags('server-NgJxxvg8OGwwBsWVXqyxQbdiflbhvugy'); + $rawFlags = $this->getFlags('server-VVhLULXCxxY0xqmszXouXxiEzoeJWmSh'); $this->flags = createFlagsFromArray($rawFlags); } @@ -570,6 +570,69 @@ public function testSetDoesNotContainAny() $this->assertEquals('on', $variant->key); } + public function testSetIsWithJsonArrayString() + { + $user = $this->userContext(null, null, null, ['key' => '["1", "2", "3"]']); + $results = $this->engine->evaluate($user, $this->flags); + $result = $results['test-set-is']; + $variant = Variant::convertEvaluationVariantToVariant($result); + $this->assertEquals('on', $variant->key); + } + + public function testIsWithArray() + { + $user = $this->userContext(null, null, null, ['key' => ['value1', 'value2']]); + $results = $this->engine->evaluate($user, $this->flags); + $result = $results['test-is-array']; + $variant = Variant::convertEvaluationVariantToVariant($result); + $this->assertEquals('on', $variant->key); + } + + public function testIsNotWithArray() + { + $user = $this->userContext(null, null, null, ['key' => ['value3', 'value4']]); + $results = $this->engine->evaluate($user, $this->flags); + $result = $results['test-is-not-array']; + $variant = Variant::convertEvaluationVariantToVariant($result); + $this->assertEquals('on', $variant->key); + } + + public function testContainsWithArray() + { + $user = $this->userContext(null, null, null, ['key' => ['has-target-value', 'has', 'value']]); + $results = $this->engine->evaluate($user, $this->flags); + $result = $results['test-contains-array']; + $variant = Variant::convertEvaluationVariantToVariant($result); + $this->assertEquals('on', $variant->key); + } + + public function testDoesNotContainWithArray() + { + $user = $this->userContext(null, null, null, ['key' => ['has-value', 'has', 'value']]); + $results = $this->engine->evaluate($user, $this->flags); + $result = $results['test-does-not-contain-array']; + $variant = Variant::convertEvaluationVariantToVariant($result); + $this->assertEquals('on', $variant->key); + } + + public function testIsWithJsonArrayString() + { + $user = $this->userContext(null, null, null, ['key' => '["value1", "value2"]']); + $results = $this->engine->evaluate($user, $this->flags); + $result = $results['test-is-array']; + $variant = Variant::convertEvaluationVariantToVariant($result); + $this->assertEquals('on', $variant->key); + } + + public function testDoesNotContainWithJsonArrayString() + { + $user = $this->userContext(null, null, null, ['key' => '["has-value", "has", "value"]']); + $results = $this->engine->evaluate($user, $this->flags); + $result = $results['test-does-not-contain-array']; + $variant = Variant::convertEvaluationVariantToVariant($result); + $this->assertEquals('on', $variant->key); + } + public function testGlobMatch() { $user = $this->userContext(null, null, null, ['key' => '/path/1/2/3/end']); diff --git a/tests/EvaluationCore/EvaluationEngineTest.php b/tests/EvaluationCore/EvaluationEngineTest.php index 52578c8..d9e1058 100644 --- a/tests/EvaluationCore/EvaluationEngineTest.php +++ b/tests/EvaluationCore/EvaluationEngineTest.php @@ -253,4 +253,84 @@ public function testCaseInsensitiveBooleanMatching() $this->assertEquals('off', $results['test-case-bool']->key, "Non-boolean value should match default segment"); } + + public function testMultiValueArrayPropertyMatching() + { + // Scalar string cases (regression coverage) + $this->assertMatch('hello', 'is', ['hello']); + $this->assertMatch('hello', 'contains', ['ell']); + $this->assertMatch('2', 'greater', ['1']); + $this->assertNoMatch('world', 'is', ['hello']); + + // Non-string scalar cases + $this->assertMatch(42, 'greater', ['1']); + $this->assertMatch(true, 'is', ['true']); + + // JSON array string + set operator + $this->assertMatch('["a","b"]', 'set contains', ['a']); + + // JSON array string + non-set operator (NEW behavior) + $this->assertMatch('["a","b"]', 'is', ['a']); + + // Collection + set operator + $this->assertMatch(['a', 'b'], 'set contains', ['a']); + + // Collection + non-set operator (NEW behavior) + $this->assertMatch(['a', 'b'], 'is', ['a']); + + // Malformed JSON array -- falls through to scalar matchString + $this->assertMatch('[broken', 'is', ['[broken']); + + // Empty JSON array + set operator + $this->assertNoMatch('[]', 'set contains', ['a']); + + // Leading whitespace prevents array parsing -- treated as scalar + $this->assertMatch(' ["a"]', 'is', [' ["a"]']); + $this->assertNoMatch(' ["a"]', 'set contains', ['a']); + + // Falsy string elements ("0", "") are preserved (only nulls are dropped) + $this->assertMatch(['0', 'a'], 'is', ['0']); + $this->assertMatch(['', 'a'], 'is', ['']); + } + + private function flagWithCondition(string $op, array $values): EvaluationFlag + { + $variants = ['on' => new EvaluationVariant('on')]; + $segment = new EvaluationSegment( + null, + [[new EvaluationCondition( + ['context', 'user', 'user_properties', 'test_prop'], + $op, + $values + )]], + 'on' + ); + return new EvaluationFlag('test-flag', $variants, [$segment]); + } + + private function contextWithProp($value): array + { + return ['user' => ['user_properties' => ['test_prop' => $value]]]; + } + + private function evaluateProp($propValue, string $op, array $values): ?EvaluationVariant + { + $flag = $this->flagWithCondition($op, $values); + $context = $this->contextWithProp($propValue); + $results = $this->engine->evaluate($context, ['test-flag' => $flag]); + return $results['test-flag'] ?? null; + } + + private function assertMatch($propValue, string $op, array $values): void + { + $result = $this->evaluateProp($propValue, $op, $values); + $this->assertNotNull($result, "Expected match, got null"); + $this->assertEquals('on', $result->key); + } + + private function assertNoMatch($propValue, string $op, array $values): void + { + $result = $this->evaluateProp($propValue, $op, $values); + $this->assertNull($result, "Expected no match, got variant"); + } }