From 265f169d14a17c351f2a700f44be8bd0157b46fa Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Thu, 23 Apr 2026 13:23:50 -0700 Subject: [PATCH 1/6] test: add harness for operator + property-value combinations --- tests/EvaluationCore/EvaluationEngineTest.php | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/EvaluationCore/EvaluationEngineTest.php b/tests/EvaluationCore/EvaluationEngineTest.php index 52578c8..d861a79 100644 --- a/tests/EvaluationCore/EvaluationEngineTest.php +++ b/tests/EvaluationCore/EvaluationEngineTest.php @@ -253,4 +253,45 @@ public function testCaseInsensitiveBooleanMatching() $this->assertEquals('off', $results['test-case-bool']->key, "Non-boolean value should match default segment"); } + + 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"); + } } From d55f18c5225b8d2d431efb7e2cb65f17b8d1f820 Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Thu, 23 Apr 2026 13:24:19 -0700 Subject: [PATCH 2/6] test: add failing cases for array-valued non-set operator matching --- tests/EvaluationCore/EvaluationEngineTest.php | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/EvaluationCore/EvaluationEngineTest.php b/tests/EvaluationCore/EvaluationEngineTest.php index d861a79..09d797a 100644 --- a/tests/EvaluationCore/EvaluationEngineTest.php +++ b/tests/EvaluationCore/EvaluationEngineTest.php @@ -254,6 +254,41 @@ public function testCaseInsensitiveBooleanMatching() "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']); + } + private function flagWithCondition(string $op, array $values): EvaluationFlag { $variants = ['on' => new EvaluationVariant('on')]; From 425046522301c9fa13edf2e0ec31703aff0642f5 Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Thu, 23 Apr 2026 13:28:04 -0700 Subject: [PATCH 3/6] feat(evaluation): any-match semantics for array-valued non-set operators Restructure matchCondition so coerceStringArray runs for every non-null, non-boolean value. Set operators keep matchSet semantics; non-set operators against array-valued properties now delegate to a new matchStringsNonSet helper that returns true if any element satisfies the predicate -- matching Amplitude analytics behavior. Scalar strings short-circuit via a strncmp prefix check in coerceStringArray to avoid json_decode overhead on every eval. --- src/EvaluationCore/EvaluationEngine.php | 50 +++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/src/EvaluationCore/EvaluationEngine.php b/src/EvaluationCore/EvaluationEngine.php index f76d819..a89f32f 100644 --- a/src/EvaluationCore/EvaluationEngine.php +++ b/src/EvaluationCore/EvaluationEngine.php @@ -117,29 +117,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 +301,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 @@ -438,6 +455,9 @@ private function coerceStringArray($value): ?array return array_filter(array_map([$this, 'coerceString'], $value)); } $stringValue = strval($value); + if (strncmp($stringValue, '[', 1) !== 0) { + return null; + } try { $parsedValue = json_decode($stringValue, true); if (is_array($parsedValue)) { From 52c3fdf9addc987b6c3dd3aa4f13f03cde2f79fd Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Thu, 23 Apr 2026 13:28:42 -0700 Subject: [PATCH 4/6] test: switch integration deployment key to VVhLULX superset --- tests/EvaluationCore/EvaluateIntegrationTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/EvaluationCore/EvaluateIntegrationTest.php b/tests/EvaluationCore/EvaluateIntegrationTest.php index fe14d31..a0d59f2 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); } From 3e560c99a402ba795c7876fa7f508eab81670585 Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Thu, 23 Apr 2026 13:29:10 -0700 Subject: [PATCH 5/6] test: integration coverage for array-valued property operators --- .../EvaluateIntegrationTest.php | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/EvaluationCore/EvaluateIntegrationTest.php b/tests/EvaluationCore/EvaluateIntegrationTest.php index a0d59f2..f71a997 100644 --- a/tests/EvaluationCore/EvaluateIntegrationTest.php +++ b/tests/EvaluationCore/EvaluateIntegrationTest.php @@ -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']); From 2045fa9037d3e572a2cc37190c025d8033964f61 Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Thu, 23 Apr 2026 13:36:38 -0700 Subject: [PATCH 6/6] refactor(evaluation): preserve falsy-string elements in coerceStringArray Address code review feedback: - coerceStringArray now drops only null elements (matching the spec's filterNotNull semantic). Previously array_filter without a callback silently dropped "0" and "" as well, which the new any-match path would have made more user-visible. - Remove the unreachable try/catch around json_decode (PHP returns null on error by default; the is_array check already handles it). Drop the now-unused Exception import. - Add two unit assertions pinning the new falsy-string behavior. --- src/EvaluationCore/EvaluationEngine.php | 25 +++++++++++-------- tests/EvaluationCore/EvaluationEngineTest.php | 4 +++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/EvaluationCore/EvaluationEngine.php b/src/EvaluationCore/EvaluationEngine.php index a89f32f..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 { @@ -452,22 +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); if (strncmp($stringValue, '[', 1) !== 0) { return null; } - try { - $parsedValue = json_decode($stringValue, true); - if (is_array($parsedValue)) { - return array_filter(array_map([$this, 'coerceString'], $parsedValue)); - } else { - return null; - } - } catch (Exception $e) { + $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/EvaluationEngineTest.php b/tests/EvaluationCore/EvaluationEngineTest.php index 09d797a..d9e1058 100644 --- a/tests/EvaluationCore/EvaluationEngineTest.php +++ b/tests/EvaluationCore/EvaluationEngineTest.php @@ -287,6 +287,10 @@ public function testMultiValueArrayPropertyMatching() // 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