Skip to content

Commit 1f6cd8b

Browse files
eli-darklyeli-darkly
authored
Merge pull request #40 from launchdarkly/eb/ch35189-35206/stringify-attrs
coerce user attributes to strings when necessary, don't send events without valid users
2 parents 87dadb3 + 2d3120b commit 1f6cd8b

File tree

5 files changed

+90
-19
lines changed

5 files changed

+90
-19
lines changed

‎src/LaunchDarkly/EventSerializer.php

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ private function filterEvent($e)
3737
return $ret;
3838
}
3939

40-
private function filterAttrs($attrs, &$json, $userPrivateAttrs, &$allPrivateAttrs)
40+
private function filterAttrs($attrs, &$json, $userPrivateAttrs, &$allPrivateAttrs, $stringify)
4141
{
4242
foreach ($attrs as $key => $value) {
4343
if ($value != null) {
@@ -46,15 +46,15 @@ private function filterAttrs($attrs, &$json, $userPrivateAttrs, &$allPrivateAttr
4646
array_search($key, $this->_privateAttrNames) !== false) {
4747
$allPrivateAttrs[$key] = true;
4848
} else {
49-
$json[$key] = $value;
49+
$json[$key] = $stringify ? strval($value) : $value;
5050
}
5151
}
5252
}
5353
}
5454

5555
private function serializeUser($user)
5656
{
57-
$json = array("key" => $user->getKey());
57+
$json = array("key" => strval($user->getKey()));
5858
$userPrivateAttrs = $user->getPrivateAttributeNames();
5959
$allPrivateAttrs = array();
6060

@@ -66,13 +66,15 @@ private function serializeUser($user)
6666
'name' => $user->getName(),
6767
'avatar' => $user->getAvatar(),
6868
'firstName' => $user->getFirstName(),
69-
'lastName' => $user->getLastName(),
70-
'anonymous' => $user->getAnonymous()
69+
'lastName' => $user->getLastName()
7170
);
72-
$this->filterAttrs($attrs, $json, $userPrivateAttrs, $allPrivateAttrs);
71+
$this->filterAttrs($attrs, $json, $userPrivateAttrs, $allPrivateAttrs, true);
72+
if ($user->getAnonymous()) {
73+
$json['anonymous'] = true;
74+
}
7375
if (!empty($user->getCustom())) {
7476
$customs = array();
75-
$this->filterAttrs($user->getCustom(), $customs, $userPrivateAttrs, $allPrivateAttrs);
77+
$this->filterAttrs($user->getCustom(), $customs, $userPrivateAttrs, $allPrivateAttrs, false);
7678
if ($customs) { // if this is empty, we will return a json array for 'custom' instead of an object
7779
$json['custom'] = $customs;
7880
}

‎src/LaunchDarkly/LDClient.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ public function track($eventName, $user, $data)
270270
}
271271
if (is_null($user) || $user->isKeyBlank()) {
272272
$this->_logger->warning("Track called with null user or null/empty user key!");
273+
return;
273274
}
274275

275276
$event = array();
@@ -293,13 +294,14 @@ public function identify($user)
293294
}
294295
if (is_null($user) || $user->isKeyBlank()) {
295296
$this->_logger->warning("Track called with null user or null/empty user key!");
297+
return;
296298
}
297299

298300
$event = array();
299301
$event['user'] = $user;
300302
$event['kind'] = "identify";
301303
$event['creationDate'] = Util::currentTimeUnixMillis();
302-
$event['key'] = $user->getKey();
304+
$event['key'] = strval($user->getKey());
303305
$this->_eventProcessor->enqueue($event);
304306
}
305307

‎src/LaunchDarkly/VariationOrRollout.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public static function bucketUser($user, $_key, $attr, $_salt)
8484
if (is_string($userValue)) {
8585
$idHash = $userValue;
8686
if ($user->getSecondary() !== null) {
87-
$idHash = $idHash . "." . $user->getSecondary();
87+
$idHash = $idHash . "." . strval($user->getSecondary());
8888
}
8989
$hash = substr(sha1($_key . "." . $_salt . "." . $idHash), 0, 15);
9090
$longVal = base_convert($hash, 16, 10);

‎tests/EventSerializerTest.php

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public function testUserKey()
126126
$builder = new LDUserBuilder("foo@bar.com");
127127
$user = $builder->build();
128128
$json = $this->getJsonForUserBySerializingEvent($user);
129-
$this->assertEquals("foo@bar.com", $json['key']);
129+
$this->assertSame("foo@bar.com", $json['key']);
130130
}
131131

132132
public function testEmptyCustom()
@@ -150,70 +150,106 @@ public function testUserSecondary()
150150
$builder = new LDUserBuilder("foo@bar.com");
151151
$user = $builder->secondary("secondary")->build();
152152
$json = $this->getJsonForUserBySerializingEvent($user);
153-
$this->assertEquals("secondary", $json['secondary']);
153+
$this->assertSame("secondary", $json['secondary']);
154154
}
155155

156156
public function testUserIP()
157157
{
158158
$builder = new LDUserBuilder("foo@bar.com");
159159
$user = $builder->ip("127.0.0.1")->build();
160160
$json = $this->getJsonForUserBySerializingEvent($user);
161-
$this->assertEquals("127.0.0.1", $json['ip']);
161+
$this->assertSame("127.0.0.1", $json['ip']);
162162
}
163163

164164
public function testUserCountry()
165165
{
166166
$builder = new LDUserBuilder("foo@bar.com");
167167
$user = $builder->country("US")->build();
168168
$json = $this->getJsonForUserBySerializingEvent($user);
169-
$this->assertEquals("US", $json['country']);
169+
$this->assertSame("US", $json['country']);
170170
}
171171

172172
public function testUserEmail()
173173
{
174174
$builder = new LDUserBuilder("foo@bar.com");
175175
$user = $builder->email("foo+test@bar.com")->build();
176176
$json = $this->getJsonForUserBySerializingEvent($user);
177-
$this->assertEquals("foo+test@bar.com", $json['email']);
177+
$this->assertSame("foo+test@bar.com", $json['email']);
178178
}
179179

180180
public function testUserName()
181181
{
182182
$builder = new LDUserBuilder("foo@bar.com");
183183
$user = $builder->name("Foo Bar")->build();
184184
$json = $this->getJsonForUserBySerializingEvent($user);
185-
$this->assertEquals("Foo Bar", $json['name']);
185+
$this->assertSame("Foo Bar", $json['name']);
186186
}
187187

188188
public function testUserAvatar()
189189
{
190190
$builder = new LDUserBuilder("foo@bar.com");
191191
$user = $builder->avatar("http://www.gravatar.com/avatar/1")->build();
192192
$json = $this->getJsonForUserBySerializingEvent($user);
193-
$this->assertEquals("http://www.gravatar.com/avatar/1", $json['avatar']);
193+
$this->assertSame("http://www.gravatar.com/avatar/1", $json['avatar']);
194194
}
195195

196196
public function testUserFirstName()
197197
{
198198
$builder = new LDUserBuilder("foo@bar.com");
199199
$user = $builder->firstName("Foo")->build();
200200
$json = $this->getJsonForUserBySerializingEvent($user);
201-
$this->assertEquals("Foo", $json['firstName']);
201+
$this->assertSame("Foo", $json['firstName']);
202202
}
203203

204204
public function testUserLastName()
205205
{
206206
$builder = new LDUserBuilder("foo@bar.com");
207207
$user = $builder->lastName("Bar")->build();
208208
$json = $this->getJsonForUserBySerializingEvent($user);
209-
$this->assertEquals("Bar", $json['lastName']);
209+
$this->assertSame("Bar", $json['lastName']);
210210
}
211211

212212
public function testUserAnonymous()
213213
{
214214
$builder = new LDUserBuilder("foo@bar.com");
215215
$user = $builder->anonymous(true)->build();
216216
$json = $this->getJsonForUserBySerializingEvent($user);
217-
$this->assertEquals(true, $json['anonymous']);
217+
$this->assertSame(true, $json['anonymous']);
218+
}
219+
220+
public function testUserNotAnonymous()
221+
{
222+
$builder = new LDUserBuilder("foo@bar.com");
223+
$user = $builder->anonymous(false)->build();
224+
$json = $this->getJsonForUserBySerializingEvent($user);
225+
$this->assertFalse(isset($json['anonymous'])); // omitted rather than set to false, for efficiency
226+
}
227+
228+
public function testNonStringAttributes()
229+
{
230+
$builder = new LDUserBuilder(1);
231+
$user = $builder->secondary(2)
232+
->ip(3)
233+
->country(4)
234+
->email(5)
235+
->name(6)
236+
->avatar(7)
237+
->firstName(8)
238+
->lastName(9)
239+
->anonymous(true)
240+
->customAttribute('foo', 10)
241+
->build();
242+
$json = $this->getJsonForUserBySerializingEvent($user);
243+
$this->assertSame('1', $json['key']);
244+
$this->assertSame('2', $json['secondary']);
245+
$this->assertSame('3', $json['ip']);
246+
$this->assertSame('4', $json['country']);
247+
$this->assertSame('5', $json['email']);
248+
$this->assertSame('6', $json['name']);
249+
$this->assertSame('7', $json['avatar']);
250+
$this->assertSame('8', $json['firstName']);
251+
$this->assertSame('9', $json['lastName']);
252+
$this->assertSame(true, $json['anonymous']); // We do NOT want "anonymous" to be stringified
253+
$this->assertSame(10, $json['custom']['foo']); // We do NOT want custom attribute values to be stringified
218254
}
219255
}

‎tests/FeatureFlagTest.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,37 @@ public function testFlagReturnsErrorIfRuleHasRolloutWithNoVariations()
615615
self::assertEquals(array(), $result->getPrerequisiteEvents());
616616
}
617617

618+
public function testSecondaryKeyIsCoercedToStringForRolloutCalculation()
619+
{
620+
// We can't really verify that the rollout calculation works correctly, but we can at least
621+
// make sure it doesn't error out if there's a non-string secondary value (ch35189)
622+
$flag = $this->makeBooleanFlagWithRules(array(
623+
array(
624+
'id' => 'ruleid',
625+
'clauses' => array(
626+
array('attribute' => 'key', 'op' => 'in', 'values' => array('userkey'), 'negate' => false)
627+
),
628+
'rollout' => array(
629+
'salt' => '',
630+
'variations' => array(
631+
array(
632+
'weight' => 100000,
633+
'variation' => 1
634+
)
635+
)
636+
)
637+
)
638+
));
639+
$ub = new LDUserBuilder('userkey');
640+
$ub->secondary(999);
641+
$user = $ub->build();
642+
643+
$result = $flag->evaluate($user, null);
644+
$detail = new EvaluationDetail(true, 1, EvaluationReason::ruleMatch(0, 'ruleid'));
645+
self::assertEquals($detail, $result->getDetail());
646+
self::assertEquals(array(), $result->getPrerequisiteEvents());
647+
}
648+
618649
public function clauseCanMatchBuiltInAttribute()
619650
{
620651
$clause = array('attribute' => 'name', 'op' => 'in', 'values' => array('Bob'), 'negate' => false);

0 commit comments

Comments
 (0)