diff --git a/.circleci/config.yml b/.circleci/config.yml index afd5166b2..44c2500d6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -84,7 +84,7 @@ jobs: sudo apt-get -q update && sudo apt-cache policy docker-ce && sudo apt-get -qy install docker-ce - - run: sudo apt-get -qy install redis-server + - run: sudo apt-get -qy install redis-server curl - checkout - run: name: validate composer.json @@ -109,6 +109,13 @@ jobs: name: start Consul command: ./consul agent -dev background: true + - run: + name: wait on Consul + command: | + until $(curl --output /dev/null --silent --fail --request PUT --data 'test' http://localhost:8500/v1/kv/initchecker); do + echo 'still waiting on Consul...'; sleep 2; + done + timeout: 50 - run: name: run tests command: vendor/bin/phpunit --log-junit ~/phpunit/junit.xml --coverage-text tests diff --git a/CHANGELOG.md b/CHANGELOG.md index 429fb0480..a7d35d6a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to the LaunchDarkly PHP SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [3.5.1] - 2019-04-03 +### Fixed: +- Setting user attributes to non-string values when a string was expected would cause analytics events not to be processed. The SDK will now convert attribute values to strings as needed. +- If `track` or `identify` is called without a user, the SDK now logs a warning, and does not send an analytics event to LaunchDarkly (since it would not be processed without a user). + ## [3.5.0] - 2019-01-30 ### Added: - It is now possible to use Consul or DynamoDB as a data store with `ld-relay`, similar to the existing Redis integration. See `LaunchDarkly\Integrations\Consul` and `LaunchDarkly\Integrations\DynamoDb`, and the reference guide [Using a persistent feature store](https://docs.launchdarkly.com/v2.0/docs/using-a-persistent-feature-store). diff --git a/VERSION b/VERSION index 1545d9665..d5c0c9914 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.5.0 +3.5.1 diff --git a/src/LaunchDarkly/EventSerializer.php b/src/LaunchDarkly/EventSerializer.php index 85b6c616f..6afea6f5d 100644 --- a/src/LaunchDarkly/EventSerializer.php +++ b/src/LaunchDarkly/EventSerializer.php @@ -37,7 +37,7 @@ private function filterEvent($e) return $ret; } - private function filterAttrs($attrs, &$json, $userPrivateAttrs, &$allPrivateAttrs) + private function filterAttrs($attrs, &$json, $userPrivateAttrs, &$allPrivateAttrs, $stringify) { foreach ($attrs as $key => $value) { if ($value != null) { @@ -46,7 +46,7 @@ private function filterAttrs($attrs, &$json, $userPrivateAttrs, &$allPrivateAttr array_search($key, $this->_privateAttrNames) !== false) { $allPrivateAttrs[$key] = true; } else { - $json[$key] = $value; + $json[$key] = $stringify ? strval($value) : $value; } } } @@ -54,7 +54,7 @@ private function filterAttrs($attrs, &$json, $userPrivateAttrs, &$allPrivateAttr private function serializeUser($user) { - $json = array("key" => $user->getKey()); + $json = array("key" => strval($user->getKey())); $userPrivateAttrs = $user->getPrivateAttributeNames(); $allPrivateAttrs = array(); @@ -66,13 +66,15 @@ private function serializeUser($user) 'name' => $user->getName(), 'avatar' => $user->getAvatar(), 'firstName' => $user->getFirstName(), - 'lastName' => $user->getLastName(), - 'anonymous' => $user->getAnonymous() + 'lastName' => $user->getLastName() ); - $this->filterAttrs($attrs, $json, $userPrivateAttrs, $allPrivateAttrs); + $this->filterAttrs($attrs, $json, $userPrivateAttrs, $allPrivateAttrs, true); + if ($user->getAnonymous()) { + $json['anonymous'] = true; + } if (!empty($user->getCustom())) { $customs = array(); - $this->filterAttrs($user->getCustom(), $customs, $userPrivateAttrs, $allPrivateAttrs); + $this->filterAttrs($user->getCustom(), $customs, $userPrivateAttrs, $allPrivateAttrs, false); if ($customs) { // if this is empty, we will return a json array for 'custom' instead of an object $json['custom'] = $customs; } diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index 4e2fbc9f3..7c45fab5e 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -270,6 +270,7 @@ public function track($eventName, $user, $data) } if (is_null($user) || $user->isKeyBlank()) { $this->_logger->warning("Track called with null user or null/empty user key!"); + return; } $event = array(); @@ -293,13 +294,14 @@ public function identify($user) } if (is_null($user) || $user->isKeyBlank()) { $this->_logger->warning("Track called with null user or null/empty user key!"); + return; } $event = array(); $event['user'] = $user; $event['kind'] = "identify"; $event['creationDate'] = Util::currentTimeUnixMillis(); - $event['key'] = $user->getKey(); + $event['key'] = strval($user->getKey()); $this->_eventProcessor->enqueue($event); } diff --git a/src/LaunchDarkly/VariationOrRollout.php b/src/LaunchDarkly/VariationOrRollout.php index df9407ea8..cf33b84ea 100644 --- a/src/LaunchDarkly/VariationOrRollout.php +++ b/src/LaunchDarkly/VariationOrRollout.php @@ -84,7 +84,7 @@ public static function bucketUser($user, $_key, $attr, $_salt) if (is_string($userValue)) { $idHash = $userValue; if ($user->getSecondary() !== null) { - $idHash = $idHash . "." . $user->getSecondary(); + $idHash = $idHash . "." . strval($user->getSecondary()); } $hash = substr(sha1($_key . "." . $_salt . "." . $idHash), 0, 15); $longVal = base_convert($hash, 16, 10); diff --git a/tests/EventSerializerTest.php b/tests/EventSerializerTest.php index 6ccdc2ee2..7d8a82f42 100644 --- a/tests/EventSerializerTest.php +++ b/tests/EventSerializerTest.php @@ -126,7 +126,7 @@ public function testUserKey() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("foo@bar.com", $json['key']); + $this->assertSame("foo@bar.com", $json['key']); } public function testEmptyCustom() @@ -150,7 +150,7 @@ public function testUserSecondary() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->secondary("secondary")->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("secondary", $json['secondary']); + $this->assertSame("secondary", $json['secondary']); } public function testUserIP() @@ -158,7 +158,7 @@ public function testUserIP() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->ip("127.0.0.1")->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("127.0.0.1", $json['ip']); + $this->assertSame("127.0.0.1", $json['ip']); } public function testUserCountry() @@ -166,7 +166,7 @@ public function testUserCountry() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->country("US")->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("US", $json['country']); + $this->assertSame("US", $json['country']); } public function testUserEmail() @@ -174,7 +174,7 @@ public function testUserEmail() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->email("foo+test@bar.com")->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("foo+test@bar.com", $json['email']); + $this->assertSame("foo+test@bar.com", $json['email']); } public function testUserName() @@ -182,7 +182,7 @@ public function testUserName() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->name("Foo Bar")->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("Foo Bar", $json['name']); + $this->assertSame("Foo Bar", $json['name']); } public function testUserAvatar() @@ -190,7 +190,7 @@ public function testUserAvatar() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->avatar("http://www.gravatar.com/avatar/1")->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("http://www.gravatar.com/avatar/1", $json['avatar']); + $this->assertSame("http://www.gravatar.com/avatar/1", $json['avatar']); } public function testUserFirstName() @@ -198,7 +198,7 @@ public function testUserFirstName() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->firstName("Foo")->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("Foo", $json['firstName']); + $this->assertSame("Foo", $json['firstName']); } public function testUserLastName() @@ -206,7 +206,7 @@ public function testUserLastName() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->lastName("Bar")->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals("Bar", $json['lastName']); + $this->assertSame("Bar", $json['lastName']); } public function testUserAnonymous() @@ -214,6 +214,42 @@ public function testUserAnonymous() $builder = new LDUserBuilder("foo@bar.com"); $user = $builder->anonymous(true)->build(); $json = $this->getJsonForUserBySerializingEvent($user); - $this->assertEquals(true, $json['anonymous']); + $this->assertSame(true, $json['anonymous']); + } + + public function testUserNotAnonymous() + { + $builder = new LDUserBuilder("foo@bar.com"); + $user = $builder->anonymous(false)->build(); + $json = $this->getJsonForUserBySerializingEvent($user); + $this->assertFalse(isset($json['anonymous'])); // omitted rather than set to false, for efficiency + } + + public function testNonStringAttributes() + { + $builder = new LDUserBuilder(1); + $user = $builder->secondary(2) + ->ip(3) + ->country(4) + ->email(5) + ->name(6) + ->avatar(7) + ->firstName(8) + ->lastName(9) + ->anonymous(true) + ->customAttribute('foo', 10) + ->build(); + $json = $this->getJsonForUserBySerializingEvent($user); + $this->assertSame('1', $json['key']); + $this->assertSame('2', $json['secondary']); + $this->assertSame('3', $json['ip']); + $this->assertSame('4', $json['country']); + $this->assertSame('5', $json['email']); + $this->assertSame('6', $json['name']); + $this->assertSame('7', $json['avatar']); + $this->assertSame('8', $json['firstName']); + $this->assertSame('9', $json['lastName']); + $this->assertSame(true, $json['anonymous']); // We do NOT want "anonymous" to be stringified + $this->assertSame(10, $json['custom']['foo']); // We do NOT want custom attribute values to be stringified } } diff --git a/tests/FeatureFlagTest.php b/tests/FeatureFlagTest.php index f0f09f02e..6546f1868 100644 --- a/tests/FeatureFlagTest.php +++ b/tests/FeatureFlagTest.php @@ -615,6 +615,37 @@ public function testFlagReturnsErrorIfRuleHasRolloutWithNoVariations() self::assertEquals(array(), $result->getPrerequisiteEvents()); } + public function testSecondaryKeyIsCoercedToStringForRolloutCalculation() + { + // We can't really verify that the rollout calculation works correctly, but we can at least + // make sure it doesn't error out if there's a non-string secondary value (ch35189) + $flag = $this->makeBooleanFlagWithRules(array( + array( + 'id' => 'ruleid', + 'clauses' => array( + array('attribute' => 'key', 'op' => 'in', 'values' => array('userkey'), 'negate' => false) + ), + 'rollout' => array( + 'salt' => '', + 'variations' => array( + array( + 'weight' => 100000, + 'variation' => 1 + ) + ) + ) + ) + )); + $ub = new LDUserBuilder('userkey'); + $ub->secondary(999); + $user = $ub->build(); + + $result = $flag->evaluate($user, null); + $detail = new EvaluationDetail(true, 1, EvaluationReason::ruleMatch(0, 'ruleid')); + self::assertEquals($detail, $result->getDetail()); + self::assertEquals(array(), $result->getPrerequisiteEvents()); + } + public function clauseCanMatchBuiltInAttribute() { $clause = array('attribute' => 'name', 'op' => 'in', 'values' => array('Bob'), 'negate' => false); diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php index 26b1900ea..25921a2cf 100644 --- a/tests/LDClientTest.php +++ b/tests/LDClientTest.php @@ -176,21 +176,21 @@ public function testVariationSendsEvent() public function testVariationDetailSendsEvent() { - $flag = $this->makeOffFlagWithValue('FUCKINGWEIRDflagkey', 'flagvalue'); - MockFeatureRequester::$flags = array('FUCKINGWEIRDflagkey' => $flag); + $flag = $this->makeOffFlagWithValue('flagkey', 'flagvalue'); + MockFeatureRequester::$flags = array('flagkey' => $flag); $client = new LDClient("someKey", array( 'feature_requester_class' => MockFeatureRequester::class, 'events' => true )); $user = new LDUser('userkey'); - $client->variationDetail('FUCKINGWEIRDflagkey', $user, 'default'); + $client->variationDetail('flagkey', $user, 'default'); $proc = $this->getPrivateField($client, '_eventProcessor'); $queue = $this->getPrivateField($proc, '_queue'); $this->assertEquals(1, sizeof($queue)); $event = $queue[0]; $this->assertEquals('feature', $event['kind']); - $this->assertEquals('FUCKINGWEIRDflagkey', $event['key']); + $this->assertEquals('flagkey', $event['key']); $this->assertEquals($flag->getVersion(), $event['version']); $this->assertEquals('flagvalue', $event['value']); $this->assertEquals(1, $event['variation']);