Skip to content

Commit 5d88c20

Browse files
eli-darklyeli-darkly
authored
(U2C #9) support contextKind in rollouts/experiments (#110)
1 parent 7f16210 commit 5d88c20

File tree

10 files changed

+145
-49
lines changed

10 files changed

+145
-49
lines changed

‎Makefile

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@ 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.
15+
# - "evaluation/bucketing/secondary": The "secondary" behavior needs to be removed from contract tests.
1416
# - "evaluation/parameterized/prerequisites": Can't pass yet because prerequisite cycle detection is not implemented.
17+
# - "evaluation/parameterized/segment match": Haven't yet implemented context kinds in segments.
18+
# - "evaluation/parameterized/segment recursion": Haven't yet implemented segment recursion.
1519
# - various other "evaluation" subtests: These tests require context kind support.
1620
# - "events": These test suites will be unavailable until more of the U2C implementation is done.
1721
TEST_HARNESS_PARAMS := $(TEST_HARNESS_PARAMS) \
18-
-skip 'evaluation/bucketing/bucket by non-key attribute' \
22+
-skip 'evaluation/bucketing/bucket by non-key attribute/in rollouts/string value/complex attribute reference' \
1923
-skip 'evaluation/bucketing/secondary' \
20-
-skip 'evaluation/bucketing/selection of context' \
2124
-skip 'evaluation/parameterized/attribute references' \
2225
-skip 'evaluation/parameterized/bad attribute reference errors' \
2326
-skip 'evaluation/parameterized/prerequisites' \

‎src/LaunchDarkly/Impl/Evaluation/Evaluator.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,6 @@ private function clauseMatchesContext(Clause $clause, LDContext $context): bool
190190
private function segmentMatchesContext(Segment $segment, LDContext $context): bool
191191
{
192192
$key = $context->getKey();
193-
if (!$key) {
194-
return false;
195-
}
196193
if (in_array($key, $segment->getIncluded(), true)) {
197194
return true;
198195
}
@@ -224,7 +221,14 @@ private function segmentRuleMatchesContext(
224221
}
225222
// All of the clauses are met. See if the user buckets in
226223
$bucketBy = $rule->getBucketBy() ?: 'key';
227-
$bucket = EvaluatorBucketing::getBucketValueForContext($context, $segmentKey, $bucketBy, $segmentSalt, null);
224+
$bucket = EvaluatorBucketing::getBucketValueForContext(
225+
$context,
226+
$rule->getRolloutContextKind(),
227+
$segmentKey,
228+
$bucketBy,
229+
$segmentSalt,
230+
null
231+
);
228232
$weight = $rule->getWeight() / 100000.0;
229233
return $bucket < $weight;
230234
}

‎src/LaunchDarkly/Impl/Evaluation/EvaluatorBucketing.php

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,30 +29,47 @@ public static function variationIndexForContext(
2929
return [null, false];
3030
}
3131
$variations = $rollout->getVariations();
32-
if ($variations) {
33-
$bucketBy = $rollout->getBucketBy() ?: "key";
34-
$bucket = self::getBucketValueForContext($context, $_key, $bucketBy, $_salt, $rollout->getSeed());
35-
$sum = 0.0;
36-
foreach ($variations as $wv) {
37-
$sum += $wv->getWeight() / 100000.0;
38-
if ($bucket < $sum) {
39-
return [$wv->getVariation(), $rollout->isExperiment() && !$wv->isUntracked()];
40-
}
32+
if (count($variations) === 0) {
33+
return [null, false];
34+
}
35+
36+
$bucketBy = ($rollout->isExperiment() ? null : $rollout->getBucketBy()) ?: 'key';
37+
$bucket = self::getBucketValueForContext(
38+
$context,
39+
$rollout->getContextKind(),
40+
$_key,
41+
$bucketBy,
42+
$_salt,
43+
$rollout->getSeed()
44+
);
45+
$experiment = $rollout->isExperiment() && $bucket >= 0;
46+
// getBucketValueForContext returns a negative value if the context didn't exist, in which case we
47+
// still end up returning the first bucket, but we will force the "in experiment" state to be false.
48+
49+
$sum = 0.0;
50+
foreach ($variations as $wv) {
51+
$sum += $wv->getWeight() / 100000.0;
52+
if ($bucket < $sum) {
53+
return [$wv->getVariation(), $experiment && !$wv->isUntracked()];
4154
}
42-
$lastVariation = $variations[count($variations) - 1];
43-
return [$lastVariation->getVariation(), $rollout->isExperiment() && !$lastVariation->isUntracked()];
4455
}
45-
return [null, false];
56+
$lastVariation = $variations[count($variations) - 1];
57+
return [$lastVariation->getVariation(), $experiment && !$lastVariation->isUntracked()];
4658
}
4759

4860
public static function getBucketValueForContext(
4961
LDContext $context,
50-
string $_key,
62+
?string $contextKind,
63+
string $key,
5164
string $attr,
52-
?string $_salt,
65+
?string $salt,
5366
?int $seed
5467
): float {
55-
$contextValue = $context->get($attr);
68+
$matchContext = $context->getIndividualContext($contextKind ?? LDContext::DEFAULT_KIND);
69+
if ($matchContext === null) {
70+
return -1;
71+
}
72+
$contextValue = $matchContext->get($attr);
5673
if ($contextValue === null) {
5774
return 0.0;
5875
}
@@ -65,7 +82,7 @@ public static function getBucketValueForContext(
6582
if (isset($seed)) {
6683
$prefix = (string) $seed;
6784
} else {
68-
$prefix = $_key . "." . ($_salt ?: '');
85+
$prefix = $key . "." . ($salt ?: '');
6986
}
7087
$hash = substr(sha1($prefix . "." . $idHash), 0, 15);
7188
$longVal = (int)base_convert($hash, 16, 10);

‎src/LaunchDarkly/Impl/Model/Rollout.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,20 @@ class Rollout
2121
private ?string $_bucketBy = null;
2222
private string $_kind;
2323
private ?int $_seed = null;
24+
private ?string $_contextKind = null;
2425

2526
public function __construct(
2627
array $variations,
2728
?string $bucketBy,
2829
?string $kind = null,
29-
?int $seed = null
30+
?int $seed = null,
31+
?string $contextKind = null
3032
) {
3133
$this->_variations = $variations;
3234
$this->_bucketBy = $bucketBy;
3335
$this->_kind = $kind ?: 'rollout';
3436
$this->_seed = $seed;
37+
$this->_contextKind = $contextKind;
3538
}
3639

3740
/**
@@ -44,7 +47,7 @@ public static function getDecoder(): \Closure
4447
$vars = array_map($decoder, $v['variations']);
4548
$bucket = $v['bucketBy'] ?? null;
4649

47-
return new Rollout($vars, $bucket, $v['kind'] ?? null, $v['seed'] ?? null);
50+
return new Rollout($vars, $bucket, $v['kind'] ?? null, $v['seed'] ?? null, $v['contextKind'] ?? null);
4851
};
4952
}
5053

@@ -70,4 +73,9 @@ public function isExperiment(): bool
7073
{
7174
return $this->_kind === self::KIND_EXPERIMENT;
7275
}
76+
77+
public function getContextKind(): ?string
78+
{
79+
return $this->_contextKind;
80+
}
7381
}

‎src/LaunchDarkly/Impl/Model/SegmentRule.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,23 @@ class SegmentRule
1818
private array $_clauses = [];
1919
private ?int $_weight = null;
2020
private ?string $_bucketBy = null;
21+
private ?string $_rolloutContextKind = null;
2122

22-
public function __construct(array $clauses, ?int $weight, ?string $bucketBy)
23+
public function __construct(array $clauses, ?int $weight, ?string $bucketBy, ?string $rolloutContextKind)
2324
{
2425
$this->_clauses = $clauses;
2526
$this->_weight = $weight;
2627
$this->_bucketBy = $bucketBy;
28+
$this->_rolloutContextKind = $rolloutContextKind;
2729
}
2830

2931
public static function getDecoder(): \Closure
3032
{
3133
return fn (array $v) => new SegmentRule(
3234
array_map(Clause::getDecoder(), $v['clauses'] ?: []),
3335
$v['weight'] ?? null,
34-
$v['bucketBy'] ?? null
36+
$v['bucketBy'] ?? null,
37+
$v['rolloutContextKind'] ?? null
3538
);
3639
}
3740

@@ -48,6 +51,11 @@ public function getBucketBy(): ?string
4851
return $this->_bucketBy;
4952
}
5053

54+
public function getRolloutContextKind(): ?string
55+
{
56+
return $this->_rolloutContextKind;
57+
}
58+
5159
public function getWeight(): ?int
5260
{
5361
return $this->_weight;

‎tests/Impl/Evaluation/EvaluatorBucketingTest.php

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ public function testUsingSeedIsDifferentThanSalt()
1515
$key = 'flag-key';
1616
$attr = 'key';
1717
$salt = 'testing123';
18-
$contextPoint1 = EvaluatorBucketing::getBucketValueForContext($context, $key, $attr, $salt, null);
19-
$contextPoint2 = EvaluatorBucketing::getBucketValueForContext($context, $key, $attr, $salt, $seed);
18+
$contextPoint1 = EvaluatorBucketing::getBucketValueForContext($context, null, $key, $attr, $salt, null);
19+
$contextPoint2 = EvaluatorBucketing::getBucketValueForContext($context, null, $key, $attr, $salt, $seed);
2020

2121
$this->assertNotEquals($contextPoint1, $contextPoint2);
2222
}
@@ -29,8 +29,8 @@ public function testDifferentSaltsProduceDifferentAssignment()
2929
$key = 'flag-key';
3030
$attr = 'key';
3131
$salt = 'testing123';
32-
$contextPoint1 = EvaluatorBucketing::getBucketValueForContext($context, $key, $attr, $salt, $seed1);
33-
$contextPoint2 = EvaluatorBucketing::getBucketValueForContext($context, $key, $attr, $salt, $seed2);
32+
$contextPoint1 = EvaluatorBucketing::getBucketValueForContext($context, null, $key, $attr, $salt, $seed1);
33+
$contextPoint2 = EvaluatorBucketing::getBucketValueForContext($context, null, $key, $attr, $salt, $seed2);
3434

3535
$this->assertNotEquals($contextPoint1, $contextPoint2);
3636
}
@@ -42,9 +42,38 @@ public function testSameSeedIsDeterministic()
4242
$key = 'flag-key';
4343
$attr = 'key';
4444
$salt = 'testing123';
45-
$contextPoint1 = EvaluatorBucketing::getBucketValueForContext($context, $key, $attr, $salt, $seed);
46-
$contextPoint2 = EvaluatorBucketing::getBucketValueForContext($context, $key, $attr, $salt, $seed);
45+
$contextPoint1 = EvaluatorBucketing::getBucketValueForContext($context, null, $key, $attr, $salt, $seed);
46+
$contextPoint2 = EvaluatorBucketing::getBucketValueForContext($context, null, $key, $attr, $salt, $seed);
4747

4848
$this->assertEquals($contextPoint1, $contextPoint2);
4949
}
50+
51+
public function testContextKindSelectsContext()
52+
{
53+
$seed = 357;
54+
$context1 = LDContext::create('key1');
55+
$context2 = LDContext::create('key2', 'kind2');
56+
$multi = LDContext::createMulti($context1, $context2);
57+
58+
$key = 'flag-key';
59+
$attr = 'key';
60+
$salt = 'testing123';
61+
62+
$this->assertEquals(
63+
EvaluatorBucketing::getBucketValueForContext($context1, null, $key, $attr, $salt, $seed),
64+
EvaluatorBucketing::getBucketValueForContext($context1, 'user', $key, $attr, $salt, $seed)
65+
);
66+
$this->assertEquals(
67+
EvaluatorBucketing::getBucketValueForContext($context1, null, $key, $attr, $salt, $seed),
68+
EvaluatorBucketing::getBucketValueForContext($multi, 'user', $key, $attr, $salt, $seed)
69+
);
70+
$this->assertEquals(
71+
EvaluatorBucketing::getBucketValueForContext($context2, 'kind2', $key, $attr, $salt, $seed),
72+
EvaluatorBucketing::getBucketValueForContext($multi, 'kind2', $key, $attr, $salt, $seed)
73+
);
74+
$this->assertNotEquals(
75+
EvaluatorBucketing::getBucketValueForContext($multi, 'user', $key, $attr, $salt, $seed),
76+
EvaluatorBucketing::getBucketValueForContext($multi, 'kind2', $key, $attr, $salt, $seed)
77+
);
78+
}
5079
}

‎tests/Impl/Evaluation/EvaluatorFlagTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ public function testRolloutSelectsBucket()
242242

243243
// First verify that with our test inputs, the bucket value will be greater than zero and less than 100000,
244244
// so we can construct a rollout whose second bucket just barely contains that value
245-
$bucketValue = floor(EvaluatorBucketing::getBucketValueForContext($context, $flagKey, "key", $salt, null) * 100000);
245+
$bucketValue = floor(EvaluatorBucketing::getBucketValueForContext($context, null, $flagKey, "key", $salt, null) * 100000);
246246
self::assertGreaterThan(0, $bucketValue);
247247
self::assertLessThan(100000, $bucketValue);
248248

@@ -272,7 +272,7 @@ public function testRolloutSelectsLastBucketIfBucketValueEqualsTotalWeight()
272272
$flagKey = 'flagkey';
273273
$salt = 'salt';
274274

275-
$bucketValue = floor(EvaluatorBucketing::getBucketValueForContext($context, $flagKey, "key", $salt, null) * 100000);
275+
$bucketValue = floor(EvaluatorBucketing::getBucketValueForContext($context, null, $flagKey, "key", $salt, null) * 100000);
276276

277277
// We'll construct a list of variations that stops right at the target bucket value
278278
$rollout = ModelBuilders::rolloutWithVariations(

‎tests/Impl/Evaluation/EvaluatorSegmentTest.php

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace LaunchDarkly\Tests\Impl\Evaluation;
44

55
use LaunchDarkly\Impl\Evaluation\Evaluator;
6+
use LaunchDarkly\Impl\Evaluation\EvaluatorBucketing;
67
use LaunchDarkly\Impl\Model\Segment;
78
use LaunchDarkly\LDContext;
89
use LaunchDarkly\Tests\MockFeatureRequester;
@@ -70,39 +71,48 @@ public function testMatchingRuleWithZeroRollout()
7071
public function testRolloutCalculationCanBucketByKey()
7172
{
7273
$context = LDContext::builder('userkey')->name('Bob')->build();
73-
$this->verifyRollout($context, 12551, 'test', 'salt', null);
74+
$this->verifyRollout($context, $context, 12551, 'test', 'salt', null, null);
7475
}
7576

7677
public function testRolloutCalculationCanBucketBySpecificAttribute()
7778
{
7879
$context = LDContext::builder('userkey')->name('Bob')->build();
79-
$this->verifyRollout($context, 61691, 'test', 'salt', 'name');
80+
$this->verifyRollout($context, $context, 61691, 'test', 'salt', 'name', null);
8081
}
8182

82-
private function verifyRollout(LDContext $context, int $expectedBucketValue, string $segmentKey, string $salt, ?string $bucketBy)
83-
{
83+
private function verifyRollout(
84+
LDContext $evalContext,
85+
LDContext $matchContext,
86+
int $expectedBucketValue,
87+
string $segmentKey,
88+
string $salt,
89+
?string $bucketBy,
90+
?string $rolloutContextKind
91+
) {
8492
$segmentShouldMatch = ModelBuilders::segmentBuilder($segmentKey)
8593
->salt($salt)
8694
->rule(
8795
ModelBuilders::segmentRuleBuilder()
88-
->clause(ModelBuilders::clauseMatchingContext($context))
96+
->clause(ModelBuilders::clauseMatchingContext($matchContext))
8997
->weight($expectedBucketValue + 1)
9098
->bucketBy($bucketBy)
99+
->rolloutContextKind($rolloutContextKind)
91100
->build()
92101
)
93102
->build();
94103
$segmentShouldNotMatch = ModelBuilders::segmentBuilder($segmentKey)
95104
->salt($salt)
96105
->rule(
97106
ModelBuilders::segmentRuleBuilder()
98-
->clause(ModelBuilders::clauseMatchingContext($context))
107+
->clause(ModelBuilders::clauseMatchingContext($matchContext))
99108
->weight($expectedBucketValue)
100109
->bucketBy($bucketBy)
110+
->rolloutContextKind($rolloutContextKind)
101111
->build()
102112
)
103113
->build();
104-
$this->assertTrue($this->segmentMatchesContext($segmentShouldMatch, $context));
105-
$this->assertFalse($this->segmentMatchesContext($segmentShouldNotMatch, $context));
114+
$this->assertTrue($this->segmentMatchesContext($segmentShouldMatch, $evalContext));
115+
$this->assertFalse($this->segmentMatchesContext($segmentShouldNotMatch, $evalContext));
106116
}
107117

108118
public function testMatchingRuleWithMultipleClauses()
@@ -119,6 +129,16 @@ public function testMatchingRuleWithMultipleClauses()
119129
$this->assertTrue(self::segmentMatchesContext($segment, $context));
120130
}
121131

132+
public function testRolloutUsesContextKind()
133+
{
134+
$context1 = LDContext::create('key1', 'kind1');
135+
$context2 = LDContext::create('key2', 'kind2');
136+
$multi = LDContext::createMulti($context1, $context2);
137+
$expectedBucketValue = (int)(100000 *
138+
EvaluatorBucketing::getBucketValueForContext($context2, 'kind2', 'test', 'key', 'salt', null));
139+
$this->verifyRollout($multi, $context2, $expectedBucketValue, 'test', 'salt', null, 'kind2');
140+
}
141+
122142
public function testNonMatchingRuleWithMultipleClauses()
123143
{
124144
$segment = ModelBuilders::segmentBuilder('test')

0 commit comments

Comments
 (0)