Skip to content

Commit 86a853a

Browse files
authored
(U2C #11) support attribute reference lookups in evaluations (#112)
* support attribute reference lookups in evaluations * misc fixes * move AttributeReference class and create instances of it * lint
1 parent 0e461ea commit 86a853a

File tree

7 files changed

+219
-16
lines changed

7 files changed

+219
-16
lines changed

Makefile

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,18 @@ TEMP_TEST_OUTPUT=/tmp/sse-contract-test-service.log
1111

1212
# TEST_HARNESS_PARAMS can be set to add -skip parameters for any contract tests that cannot yet pass
1313
# Explanation of current skips:
14-
# - "evaluation" subtests involving attribute references: Haven't yet implemented attribute references.
1514
# - "evaluation/bucketing/secondary": The "secondary" behavior needs to be removed from contract tests.
15+
# - "evaluation/parameterized/attribute references/array index is not supported": Due to how PHP
16+
# arrays work, there's no way to disallow an array index lookup without breaking object property
17+
# lookups for properties that are numeric strings.
1618
# - "evaluation/parameterized/prerequisites": Can't pass yet because prerequisite cycle detection is not implemented.
1719
# - "evaluation/parameterized/segment match": Haven't yet implemented context kinds in segments.
1820
# - "evaluation/parameterized/segment recursion": Haven't yet implemented segment recursion.
1921
# - various other "evaluation" subtests: These tests require context kind support.
2022
# - "events": These test suites will be unavailable until more of the U2C implementation is done.
2123
TEST_HARNESS_PARAMS := $(TEST_HARNESS_PARAMS) \
22-
-skip 'evaluation/bucketing/bucket by non-key attribute/in rollouts/string value/complex attribute reference' \
2324
-skip 'evaluation/bucketing/secondary' \
24-
-skip 'evaluation/parameterized/attribute references' \
25-
-skip 'evaluation/parameterized/bad attribute reference errors' \
25+
-skip 'evaluation/parameterized/attribute references/array index is not supported' \
2626
-skip 'evaluation/parameterized/prerequisites' \
2727
-skip 'evaluation/parameterized/segment recursion' \
2828
-skip 'events'
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LaunchDarkly\Impl\Evaluation;
6+
7+
/**
8+
* Used within the evaluation flow to short-circuit evaluation and force an error result.
9+
* This exception should never be thrown outside of the Evaluator into application code.
10+
*
11+
* @internal
12+
* @ignore
13+
*/
14+
class EvaluationException extends \Exception
15+
{
16+
private string $_errorKind;
17+
18+
public function __construct(string $message, string $errorKind)
19+
{
20+
parent::__construct($message);
21+
$this->_errorKind = $errorKind;
22+
}
23+
24+
public function getErrorKind(): string
25+
{
26+
return $this->_errorKind;
27+
}
28+
}

src/LaunchDarkly/Impl/Evaluation/Evaluator.php

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace LaunchDarkly\Impl\Evaluation;
44

5+
use LaunchDarkly\EvaluationDetail;
56
use LaunchDarkly\EvaluationReason;
67
use LaunchDarkly\FeatureRequester;
78
use LaunchDarkly\Impl\Model\Clause;
@@ -41,7 +42,11 @@ public function __construct(FeatureRequester $featureRequester)
4142
*/
4243
public function evaluate(FeatureFlag $flag, LDContext $context, ?callable $prereqEvalSink): EvalResult
4344
{
44-
return $this->evaluateInternal($flag, $context, $prereqEvalSink);
45+
try {
46+
return $this->evaluateInternal($flag, $context, $prereqEvalSink);
47+
} catch (EvaluationException $e) {
48+
return new EvalResult(new EvaluationDetail(null, null, EvaluationReason::error($e->getErrorKind())));
49+
}
4550
}
4651

4752
private function evaluateInternal(
@@ -231,14 +236,18 @@ private function segmentRuleMatchesContext(
231236
}
232237
// All of the clauses are met. See if the user buckets in
233238
$bucketBy = $rule->getBucketBy() ?: 'key';
234-
$bucket = EvaluatorBucketing::getBucketValueForContext(
235-
$context,
236-
$rule->getRolloutContextKind(),
237-
$segmentKey,
238-
$bucketBy,
239-
$segmentSalt,
240-
null
241-
);
239+
try {
240+
$bucket = EvaluatorBucketing::getBucketValueForContext(
241+
$context,
242+
$rule->getRolloutContextKind(),
243+
$segmentKey,
244+
$bucketBy,
245+
$segmentSalt,
246+
null
247+
);
248+
} catch (InvalidAttributeReferenceException $e) {
249+
return false;
250+
}
242251
$weight = $rule->getWeight() / 100000.0;
243252
return $bucket < $weight;
244253
}

src/LaunchDarkly/Impl/Evaluation/EvaluatorBucketing.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public static function getBucketValueForContext(
6969
if ($matchContext === null) {
7070
return -1;
7171
}
72-
$contextValue = $matchContext->get($attr);
72+
$contextValue = EvaluatorHelpers::getContextValueForAttributeReference($matchContext, $attr, $contextKind);
7373
if ($contextValue === null) {
7474
return 0.0;
7575
}

src/LaunchDarkly/Impl/Evaluation/EvaluatorHelpers.php

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use LaunchDarkly\EvaluationDetail;
66
use LaunchDarkly\EvaluationReason;
7+
use LaunchDarkly\Impl\Model\AttributeReference;
78
use LaunchDarkly\Impl\Model\Clause;
89
use LaunchDarkly\Impl\Model\FeatureFlag;
910
use LaunchDarkly\Impl\Model\Target;
@@ -41,6 +42,44 @@ public static function evaluationDetailForVariation(
4142
return new EvaluationDetail($vars[$index], $index, $reason);
4243
}
4344

45+
public static function getContextValueForAttributeReference(
46+
LDContext $context,
47+
string $attributeRef,
48+
?string $forContextKind
49+
): mixed {
50+
if ($attributeRef === '') {
51+
throw new InvalidAttributeReferenceException(AttributeReference::ERR_ATTR_EMPTY);
52+
}
53+
if ($forContextKind === null || $forContextKind === '') {
54+
// Treat the attribute as just an attribute name, not a reference path
55+
return $context->get($attributeRef);
56+
}
57+
$parsed = AttributeReference::parse($attributeRef);
58+
if (($err = $parsed->getError()) !== null) {
59+
throw new InvalidAttributeReferenceException($err);
60+
}
61+
$depth = $parsed->getDepth();
62+
$value = $context->get($parsed->getComponent(0));
63+
if ($depth <= 1) {
64+
return $value;
65+
}
66+
for ($i = 1; $i < $depth; $i++) {
67+
$propName = $parsed->getComponent($i);
68+
if (is_object($value)) {
69+
$value = get_object_vars($value)[$propName] ?? null;
70+
} elseif (is_array($value)) {
71+
// Note that either a JSON array or a JSON object could be represented as a PHP array.
72+
// There is no good way to distinguish between ["a", "b"] and {"0": "a", "1": "b"}.
73+
// Therefore, our lookup logic here is slightly more permissive than other SDKs, where
74+
// an attempt to get /attr/0 would only work in the second case and not in the first.
75+
$value = $value[$propName] ?? null;
76+
} else {
77+
return null;
78+
}
79+
}
80+
return $value;
81+
}
82+
4483
public static function getOffResult(FeatureFlag $flag, EvaluationReason $reason): EvalResult
4584
{
4685
$offVar = $flag->getOffVariation();
@@ -57,7 +96,11 @@ public static function getResultForVariationOrRollout(
5796
LDContext $context,
5897
EvaluationReason $reason
5998
): EvalResult {
60-
list($index, $inExperiment) = EvaluatorBucketing::variationIndexForContext($r, $context, $flag->getKey(), $flag->getSalt());
99+
try {
100+
list($index, $inExperiment) = EvaluatorBucketing::variationIndexForContext($r, $context, $flag->getKey(), $flag->getSalt());
101+
} catch (InvalidAttributeReferenceException $e) {
102+
return new EvalResult(new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)));
103+
}
61104
if ($index === null) {
62105
return new EvalResult(
63106
new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)),
@@ -90,7 +133,7 @@ public static function matchClauseWithoutSegments(Clause $clause, LDContext $con
90133
if ($actualContext === null) {
91134
return false;
92135
}
93-
$contextValue = $actualContext->get($attr);
136+
$contextValue = self::getContextValueForAttributeReference($actualContext, $attr, $clause->getContextKind());
94137
if ($contextValue === null) {
95138
return false;
96139
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LaunchDarkly\Impl\Evaluation;
6+
7+
use LaunchDarkly\EvaluationReason;
8+
9+
/**
10+
* @internal
11+
* @ignore
12+
*/
13+
class InvalidAttributeReferenceException extends EvaluationException
14+
{
15+
public function __construct(string $message)
16+
{
17+
parent::__construct($message, EvaluationReason::MALFORMED_FLAG_ERROR);
18+
}
19+
}
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace LaunchDarkly\Impl\Model;
6+
7+
/**
8+
* An attribute name or path expression identifying a value within an {@link LDContext}.
9+
*
10+
* Unlike some other SDKs, this class is not exposed in the public API. Due to the
11+
* stateless nature of typical PHP applications, there is little advantage in pre-parsing
12+
* attribute references and storing them internally. This class just encapsulates the
13+
* parsing logic.
14+
*
15+
* @ignore
16+
* @internal
17+
*/
18+
class AttributeReference
19+
{
20+
const ERR_ATTR_EMPTY = 'attribute reference cannot be empty';
21+
const ERR_ATTR_EXTRA_SLASH = 'attribute reference contained a double slash or a trailing slash';
22+
const ERR_ATTR_INVALID_ESCAPE =
23+
'attribute reference contained an escape character (~) that was not followed by 0 or 1';
24+
25+
private string $_path;
26+
private ?string $_singleComponent;
27+
/** @var string[]|null */
28+
private ?array $_components;
29+
private ?string $_error;
30+
31+
private function __construct(string $path, ?string $singleComponent, ?array $components, ?string $error)
32+
{
33+
$this->_path = $path;
34+
$this->_singleComponent = $singleComponent;
35+
$this->_components = $components;
36+
$this->_error = $error;
37+
}
38+
39+
public static function parse(string $refPath): AttributeReference
40+
{
41+
if ($refPath === '' || $refPath === '/') {
42+
return self::failed($refPath, self::ERR_ATTR_EMPTY);
43+
}
44+
if (!str_starts_with($refPath, '/')) {
45+
return new AttributeReference($refPath, $refPath, null, null);
46+
}
47+
$components = explode('/', substr($refPath, 1));
48+
if (count($components) === 1) {
49+
$attr = self::unescape($components[0]);
50+
if ($attr === null) {
51+
return self::failed($refPath, self::ERR_ATTR_INVALID_ESCAPE);
52+
}
53+
return new AttributeReference($refPath, $attr, null, null);
54+
}
55+
for ($i = 0; $i < count($components); $i++) {
56+
$prop = $components[$i];
57+
if ($prop === '') {
58+
return self::failed($refPath, self::ERR_ATTR_EXTRA_SLASH);
59+
}
60+
$prop = self::unescape($prop);
61+
if ($prop === null) {
62+
return self::failed($refPath, self::ERR_ATTR_INVALID_ESCAPE);
63+
}
64+
$components[$i] = $prop;
65+
}
66+
return new AttributeReference($refPath, null, $components, null);
67+
}
68+
69+
private static function failed(string $refPath, string $error): AttributeReference
70+
{
71+
return new AttributeReference($refPath, null, null, $error);
72+
}
73+
74+
public function getPath(): string
75+
{
76+
return $this->_path;
77+
}
78+
79+
public function getError(): ?string
80+
{
81+
return $this->_error;
82+
}
83+
84+
public function getDepth(): int
85+
{
86+
return $this->_components === null ? 1 : count($this->_components);
87+
}
88+
89+
public function getComponent(int $index): string
90+
{
91+
if ($this->_components === null) {
92+
return $index === 0 ? ($this->_singleComponent ?: '') : '';
93+
}
94+
return $index < 0 || $index >= count($this->_components) ? '' : $this->_components[$index];
95+
}
96+
97+
private static function unescape(string $s): ?string
98+
{
99+
if (preg_match('/(~[^01]|~$)/', $s)) {
100+
return null;
101+
}
102+
return str_replace('~0', '~', str_replace('~1', '/', $s));
103+
}
104+
}

0 commit comments

Comments
 (0)