From aa34e8b4a9c6a0b5dac74f65f7b5d6f12d26ef36 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 16 Jul 2018 16:05:35 -0700 Subject: [PATCH 01/17] fix version string --- CHANGELOG.md | 4 ++++ VERSION | 2 +- src/LaunchDarkly/LDClient.php | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f9560107..bf3863c9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to the LaunchDarkly PHP SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [3.2.1] - 2018-07-16 +### Fixed: +- The `LDClient::VERSION` constant has been fixed to report the current version. In the previous release, it was still set to 3.1.0. + ## [3.2.0] - 2018-06-26 ### Changed: - The client now treats most HTTP 4xx errors as unrecoverable: that is, after receiving such an error, it will take the client offline (for the lifetime of the client instance, which in most PHP applications is just the current request-response cycle). This is because such errors indicate either a configuration problem (invalid SDK key) or a bug, which is not likely to resolve without a restart or an upgrade. This does not apply if the error is 400, 408, 429, or any 5xx error. diff --git a/VERSION b/VERSION index 944880fa1..e4604e3af 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0 +3.2.1 diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index 4d2ac10b1..7ab75fe69 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -12,7 +12,7 @@ class LDClient { const DEFAULT_BASE_URI = 'https://app.launchdarkly.com'; const DEFAULT_EVENTS_URI = 'https://events.launchdarkly.com'; - const VERSION = '3.1.0'; + const VERSION = '3.2.1'; /** @var string */ protected $_sdkKey; From 0d4fc33e9a381e4efe8eccd0ba3227061fd89cec Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Aug 2018 16:12:18 -0700 Subject: [PATCH 02/17] add new version of allFlags() that captures more metadata --- src/LaunchDarkly/EvalResult.php | 2 +- src/LaunchDarkly/FeatureFlag.php | 34 ++++++- src/LaunchDarkly/FeatureFlagsState.php | 90 +++++++++++++++++++ src/LaunchDarkly/LDClient.php | 47 +++++++--- tests/LDClientTest.php | 117 +++++++++++++++++++++++-- tests/MockFeatureRequester.php | 6 +- 6 files changed, 270 insertions(+), 26 deletions(-) create mode 100644 src/LaunchDarkly/FeatureFlagsState.php diff --git a/src/LaunchDarkly/EvalResult.php b/src/LaunchDarkly/EvalResult.php index f2fff0a4b..f692abac1 100644 --- a/src/LaunchDarkly/EvalResult.php +++ b/src/LaunchDarkly/EvalResult.php @@ -22,7 +22,7 @@ public function __construct($variation, $value, array $prerequisiteEvents) } /** - * @return int + * @return int | null */ public function getVariation() { diff --git a/src/LaunchDarkly/FeatureFlag.php b/src/LaunchDarkly/FeatureFlag.php index 2c94c2498..c6b433052 100644 --- a/src/LaunchDarkly/FeatureFlag.php +++ b/src/LaunchDarkly/FeatureFlag.php @@ -27,6 +27,13 @@ class FeatureFlag protected $_variations = array(); /** @var bool */ protected $_deleted = false; + /** @var bool */ + protected $_trackEvents = false; + /** @var int | null */ + protected $_debugEventsUntilDate = null; + // Note, trackEvents and debugEventsUntilDate are not used in EventProcessor, because + // the PHP client doesn't do summary events. However, we need to capture them in case + // they want to pass the flag data to the front end with allFlagsState(). protected function __construct($key, $version, @@ -38,7 +45,9 @@ protected function __construct($key, $fallthrough, $offVariation, array $variations, - $deleted) + $deleted, + $trackEvents, + $debugEventsUntilDate) { $this->_key = $key; $this->_version = $version; @@ -51,6 +60,8 @@ protected function __construct($key, $this->_offVariation = $offVariation; $this->_variations = $variations; $this->_deleted = $deleted; + $this->_trackEvents = $trackEvents; + $this->_debugEventsUntilDate = $debugEventsUntilDate; } public static function getDecoder() @@ -67,7 +78,10 @@ public static function getDecoder() call_user_func(VariationOrRollout::getDecoder(), $v['fallthrough']), $v['offVariation'], $v['variations'] ?: [], - $v['deleted']); + $v['deleted'], + $v['trackEvents'], + $v['debugEventsUntilDate'] + ); }; } @@ -222,4 +236,20 @@ public function isDeleted() { return $this->_deleted; } + + /** + * @return boolean + */ + public function isTrackEvents() + { + return $this->_trackEvents; + } + + /** + * @return int | null + */ + public function getDebugEventsUntilDate() + { + return $this->_debugEventsUntilDate; + } } diff --git a/src/LaunchDarkly/FeatureFlagsState.php b/src/LaunchDarkly/FeatureFlagsState.php new file mode 100644 index 000000000..acf2b221c --- /dev/null +++ b/src/LaunchDarkly/FeatureFlagsState.php @@ -0,0 +1,90 @@ +_valid = $valid; + $this->_flagValues = array(); + $this->_flagMetadata = array(); + } + + /** + * Used internally to build the state map. + */ + public function addFlag($flag, $evalResult) + { + $this->_flagValues[$flag->getKey()] = $evalResult->getValue(); + $meta = array(); + if (!is_null($evalResult->getVariation())) { + $meta['variation'] = $evalResult->getVariation(); + } + $meta['version'] = $flag->getVersion(); + $meta['trackEvents'] = $flag->isTrackEvents(); + if ($flag->getDebugEventsUntilDate()) { + $meta['debugEventsUntilDate'] = $flag->getDebugEventsUntilDate(); + } + $this->_flagMetadata[$flag->getKey()] = $meta; + } + + /** + * Returns true if this object contains a valid snapshot of feature flag state, or false if the + * state could not be computed (for instance, because the client was offline or there was no user). + * @return bool true if the state is valid + */ + public function isValid() + { + return $this->_valid; + } + + /** + * Returns the value of an individual feature flag at the time the state was recorded. + * @param $key string + * @return mixed the flag's value; null if the flag returned the default value, or if there was no such flag + */ + public function getFlagValue($key) + { + return $this->_flagValues[$key]; + } + + /** + * Returns an associative array of flag keys to flag values. If a flag would have evaluated to the default + * value, its value will be null. + *

+ * Do not use this method if you are passing data to the front end to "bootstrap" the JavaScript client. + * Instead, use toJson(). + * @return array an associative array of flag keys to JSON values + */ + public function toValuesMap() + { + return $this->_flagValues; + } + + /** + * Returns a JSON representation of the entire state map (as an associative array), in the format used + * by the LaunchDarkly JavaScript SDK. Use this method if you are passing data to the front end in + * order to "bootstrap" the JavaScript client. + * + * @return array an associative array suitable for passing as a JSON object + */ + public function toJson() + { + $ret = array_replace([], $this->_flagValues); + $ret['$flagsState'] = $this->_flagMetadata; + return $ret; + } +} diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index 7ab75fe69..e86e1b874 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -261,37 +261,56 @@ public function identify($user) *

* The most common use case for this method is to bootstrap a set of client-side feature flags from a back-end service. * + * @deprecated Use allFlagsState() instead. Current versions of the client-side SDK will not + * generate analytics events correctly if you pass the result of allFlags(). * @param $user LDUser the end user requesting the feature flags * @return array()|null Mapping of feature flag keys to their evaluated results for $user */ public function allFlags($user) { - if (is_null($user) || is_null($user->getKey())) { - $this->_logger->warn("allFlags called with null user or null/empty user key! Returning null"); + $state = $this->allFlagsState($user); + if (!$state->isValid()) { return null; } + return $state->toValuesMap(); + } + + /** + * Returns an object that encapsulates the state of all feature flags for a given user, including the flag + * values and also metadata that can be used on the front end. This method does not send analytics events + * back to LaunchDarkly. + *

+ * The most common use case for this method is to bootstrap a set of client-side feature flags from a back-end service. + * To convert the state object into a JSON data structure, call its toJson() method. + * + * @param $user LDUser the end user requesting the feature flags + * @return FeatureFlagsState a FeatureFlagsState object (will never be null; see FeatureFlagsState.isValid()) + */ + public function allFlagsState($user) + { + if (is_null($user) || is_null($user->getKey())) { + $this->_logger->warn("allFlagsState called with null user or null/empty user key! Returning empty state"); + return new FeatureFlagsState(false); + } if ($this->isOffline()) { - return null; + return new FeatureFlagsState(false); } try { $flags = $this->_featureRequester->getAllFeatures(); } catch (UnrecoverableHTTPStatusException $e) { $this->handleUnrecoverableError(); - return null; + return new FeatureFlagsState(false); } if ($flags === null) { - return null; + return new FeatureFlagsState(false); } - /** - * @param $flag FeatureFlag - * @return mixed|null - */ - $eval = function ($flag) use ($user) { - return $flag->evaluate($user, $this->_featureRequester)->getValue(); - }; - - return array_map($eval, $flags); + $state = new FeatureFlagsState(true); + foreach ($flags as $key => $flag) { + $result = $flag->evaluate($user, $this->_featureRequester); + $state->addFlag($flag, $result); + } + return $state; } /** Generates an HMAC sha256 hash for use in Secure mode: https://github.com/launchdarkly/js-client#secure-mode diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php index b5b36af6c..52d5493b2 100644 --- a/tests/LDClientTest.php +++ b/tests/LDClientTest.php @@ -2,6 +2,7 @@ namespace LaunchDarkly\Tests; use InvalidArgumentException; +use LaunchDarkly\FeatureFlag; use LaunchDarkly\FeatureRequester; use LaunchDarkly\LDClient; use LaunchDarkly\LDUser; @@ -15,9 +16,38 @@ public function testDefaultCtor() $this->assertInstanceOf(LDClient::class, new LDClient("BOGUS_SDK_KEY")); } - public function testToggleDefault() + public function testVariationReturnsFlagValue() { - MockFeatureRequester::$val = null; + $flagJson = array( + 'key' => 'feature', + 'version' => 100, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => 1, + 'fallthrough' => array('variation' => 0), + 'variations' => array('fall', 'off', 'on'), + 'salt' => '' + ); + $flag = FeatureFlag::decode($flagJson); + + MockFeatureRequester::$flags = array('feature' => $flag); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $builder = new LDUserBuilder(3); + $user = $builder->build(); + $value = $client->variation('feature', $user, 'default'); + $this->assertEquals('off', $value); + } + + public function testVariationReturnsDefaultForUnknownFlag() + { + MockFeatureRequester::$flags = array(); $client = new LDClient("someKey", array( 'feature_requester_class' => MockFeatureRequester::class, 'events' => false @@ -28,9 +58,9 @@ public function testToggleDefault() $this->assertEquals('argdef', $client->variation('foo', $user, 'argdef')); } - public function testToggleFromArray() + public function testVariationReturnsDefaultFromConfigurationForUnknownFlag() { - MockFeatureRequester::$val = null; + MockFeatureRequester::$flags = array(); $client = new LDClient("someKey", array( 'feature_requester_class' => MockFeatureRequester::class, 'events' => false, @@ -42,9 +72,9 @@ public function testToggleFromArray() $this->assertEquals('fromarray', $client->variation('foo', $user, 'argdef')); } - public function testToggleEvent() + public function testVariationSendsEvent() { - MockFeatureRequester::$val = null; + MockFeatureRequester::$flags = array(); $client = new LDClient("someKey", array( 'feature_requester_class' => MockFeatureRequester::class, 'events' => true @@ -58,6 +88,81 @@ public function testToggleEvent() $this->assertEquals(1, sizeof($queue)); } + public function testAllFlagsReturnsFlagValues() + { + $flagJson = array( + 'key' => 'feature', + 'version' => 100, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => 1, + 'fallthrough' => array('variation' => 0), + 'variations' => array('fall', 'off', 'on'), + 'salt' => '' + ); + $flag = FeatureFlag::decode($flagJson); + + MockFeatureRequester::$flags = array('feature' => $flag); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $builder = new LDUserBuilder(3); + $user = $builder->build(); + $values = $client->allFlags($user); + + $this->assertEquals(array('feature' => 'off'), $values); + } + + public function testAllFlagsStateReturnsState() + { + $flagJson = array( + 'key' => 'feature', + 'version' => 100, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => 1, + 'fallthrough' => array('variation' => 0), + 'variations' => array('fall', 'off', 'on'), + 'salt' => '', + 'trackEvents' => true, + 'debugEventsUntilDate' => 1000 + ); + $flag = FeatureFlag::decode($flagJson); + + MockFeatureRequester::$flags = array('feature' => $flag); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $builder = new LDUserBuilder(3); + $user = $builder->build(); + $state = $client->allFlagsState($user); + + $this->assertTrue($state->isValid()); + $this->assertEquals(array('feature' => 'off'), $state->toValuesMap()); + $expectedState = array( + 'feature' => 'off', + '$flagsState' => array( + 'feature' => array( + 'variation' => 1, + 'version' => 100, + 'trackEvents' => true, + 'debugEventsUntilDate' => 1000 + ) + ) + ); + $this->assertEquals($expectedState, $state->toJson()); + } + public function testOnlyValidFeatureRequester() { $this->setExpectedException(InvalidArgumentException::class); diff --git a/tests/MockFeatureRequester.php b/tests/MockFeatureRequester.php index b2e6cf847..5ecae1fe8 100644 --- a/tests/MockFeatureRequester.php +++ b/tests/MockFeatureRequester.php @@ -5,7 +5,7 @@ class MockFeatureRequester implements FeatureRequester { - public static $val = null; + public static $flags = array(); public function __construct($baseurl, $key, $options) { @@ -13,7 +13,7 @@ public function __construct($baseurl, $key, $options) public function getFeature($key) { - return self::$val; + return self::$flags[$key]; } public function getSegment($key) @@ -23,6 +23,6 @@ public function getSegment($key) public function getAllFeatures() { - return null; + return self::$flags; } } From 0f769695d9580a31f06b6343855b4731f0f00454 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Aug 2018 16:16:11 -0700 Subject: [PATCH 03/17] linter --- src/LaunchDarkly/FeatureFlagsState.php | 3 +-- src/LaunchDarkly/LDClient.php | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/LaunchDarkly/FeatureFlagsState.php b/src/LaunchDarkly/FeatureFlagsState.php index acf2b221c..6e5c8ac01 100644 --- a/src/LaunchDarkly/FeatureFlagsState.php +++ b/src/LaunchDarkly/FeatureFlagsState.php @@ -35,7 +35,7 @@ public function addFlag($flag, $evalResult) } $meta['version'] = $flag->getVersion(); $meta['trackEvents'] = $flag->isTrackEvents(); - if ($flag->getDebugEventsUntilDate()) { + if ($flag->getDebugEventsUntilDate()) { $meta['debugEventsUntilDate'] = $flag->getDebugEventsUntilDate(); } $this->_flagMetadata[$flag->getKey()] = $meta; @@ -78,7 +78,6 @@ public function toValuesMap() * Returns a JSON representation of the entire state map (as an associative array), in the format used * by the LaunchDarkly JavaScript SDK. Use this method if you are passing data to the front end in * order to "bootstrap" the JavaScript client. - * * @return array an associative array suitable for passing as a JSON object */ public function toJson() diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index e86e1b874..f528d25a3 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -260,7 +260,6 @@ public function identify($user) * This method will not send analytics events back to LaunchDarkly. *

* The most common use case for this method is to bootstrap a set of client-side feature flags from a back-end service. - * * @deprecated Use allFlagsState() instead. Current versions of the client-side SDK will not * generate analytics events correctly if you pass the result of allFlags(). * @param $user LDUser the end user requesting the feature flags @@ -282,7 +281,6 @@ public function allFlags($user) *

* The most common use case for this method is to bootstrap a set of client-side feature flags from a back-end service. * To convert the state object into a JSON data structure, call its toJson() method. - * * @param $user LDUser the end user requesting the feature flags * @return FeatureFlagsState a FeatureFlagsState object (will never be null; see FeatureFlagsState.isValid()) */ From 9023ea673deaff0065c790cf5025826105c7b3ea Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Aug 2018 17:20:44 -0700 Subject: [PATCH 04/17] missing array key guard --- src/LaunchDarkly/FeatureFlagsState.php | 2 +- tests/MockFeatureRequester.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/LaunchDarkly/FeatureFlagsState.php b/src/LaunchDarkly/FeatureFlagsState.php index 6e5c8ac01..a897f5e50 100644 --- a/src/LaunchDarkly/FeatureFlagsState.php +++ b/src/LaunchDarkly/FeatureFlagsState.php @@ -58,7 +58,7 @@ public function isValid() */ public function getFlagValue($key) { - return $this->_flagValues[$key]; + return isset($this->_flagValues[$key]) ? $this->_flagValues[$key] : null; } /** diff --git a/tests/MockFeatureRequester.php b/tests/MockFeatureRequester.php index 5ecae1fe8..995ec41bc 100644 --- a/tests/MockFeatureRequester.php +++ b/tests/MockFeatureRequester.php @@ -13,7 +13,7 @@ public function __construct($baseurl, $key, $options) public function getFeature($key) { - return self::$flags[$key]; + return isset(self::$flags[$key]) ? self::$flags[$key] : null; } public function getSegment($key) From e215364dc04b6d77bcfd15a6b9ba78657f021ce5 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Aug 2018 17:27:34 -0700 Subject: [PATCH 05/17] missing array key guards --- src/LaunchDarkly/FeatureFlag.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/LaunchDarkly/FeatureFlag.php b/src/LaunchDarkly/FeatureFlag.php index c6b433052..f617cb672 100644 --- a/src/LaunchDarkly/FeatureFlag.php +++ b/src/LaunchDarkly/FeatureFlag.php @@ -79,8 +79,8 @@ public static function getDecoder() $v['offVariation'], $v['variations'] ?: [], $v['deleted'], - $v['trackEvents'], - $v['debugEventsUntilDate'] + isset($v['trackEvents']) && $v['trackEvents'], + isset($v['debugEventsUntilDate']) ? $v['debugEventsUntilDate'] : null ); }; } From 30e9e3cf8a73ef5006642cb52a800baed6f30aef Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Aug 2018 20:41:32 -0700 Subject: [PATCH 06/17] use the standard method for specifying custom JSON serialization --- src/LaunchDarkly/FeatureFlagsState.php | 14 +++- tests/FeatureFlagsStateTest.php | 111 +++++++++++++++++++++++++ tests/LDClientTest.php | 5 +- 3 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 tests/FeatureFlagsStateTest.php diff --git a/src/LaunchDarkly/FeatureFlagsState.php b/src/LaunchDarkly/FeatureFlagsState.php index a897f5e50..87c84d2b4 100644 --- a/src/LaunchDarkly/FeatureFlagsState.php +++ b/src/LaunchDarkly/FeatureFlagsState.php @@ -3,9 +3,11 @@ /** * A snapshot of the state of all feature flags with regard to a specific user, generated by - * calling LDClient.allFlagsState(). + * calling LDClient.allFlagsState(). Serializing this object to JSON using json_encode(), or + * the jsonSerialize() method, will produce the appropriate data structure for bootstrapping + * the LaunchDarkly JavaScript client. */ -class FeatureFlagsState +class FeatureFlagsState implements \JsonSerializable { /** @var bool */ protected $_valid = false; @@ -66,7 +68,7 @@ public function getFlagValue($key) * value, its value will be null. *

* Do not use this method if you are passing data to the front end to "bootstrap" the JavaScript client. - * Instead, use toJson(). + * Instead, use jsonSerialize(). * @return array an associative array of flag keys to JSON values */ public function toValuesMap() @@ -78,12 +80,16 @@ public function toValuesMap() * Returns a JSON representation of the entire state map (as an associative array), in the format used * by the LaunchDarkly JavaScript SDK. Use this method if you are passing data to the front end in * order to "bootstrap" the JavaScript client. + *

+ * Note that calling json_encode() on a FeatureFlagsState object will automatically use the + * jsonSerialize() method. * @return array an associative array suitable for passing as a JSON object */ - public function toJson() + public function jsonSerialize() { $ret = array_replace([], $this->_flagValues); $ret['$flagsState'] = $this->_flagMetadata; + $ret['$valid'] = $this->_valid; return $ret; } } diff --git a/tests/FeatureFlagsStateTest.php b/tests/FeatureFlagsStateTest.php new file mode 100644 index 000000000..f61981909 --- /dev/null +++ b/tests/FeatureFlagsStateTest.php @@ -0,0 +1,111 @@ + 'key1', + 'version' => 100, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => 0, + 'fallthrough' => array('variation' => 0), + 'variations' => array('value1'), + 'salt' => '', + 'trackEvents' => false + ); + private static $flag2Json = array( + 'key' => 'key2', + 'version' => 200, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => 0, + 'fallthrough' => array('variation' => 0), + 'variations' => array('value2'), + 'salt' => '', + 'trackEvents' => true, + 'debugEventsUntilDate' => 1000 + ); + + public function testCanGetFlagValue() + { + $flag = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag, new EvalResult(0, 'value1', array())); + + $this->assertEquals('value1', $state->getFlagValue('key1')); + } + + public function testUnknownFlagReturnsNullValue() + { + $state = new FeatureFlagsState(true); + + $this->assertNull($state->getFlagValue('key1')); + } + + public function testCanConvertToValuesMap() + { + $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag1, new EvalResult(0, 'value1', array())); + $state->addFlag($flag2, new EvalResult(0, 'value2', array())); + + $expected = array('key1' => 'value1', 'key2' => 'value2'); + $this->assertEquals($expected, $state->toValuesMap()); + } + + public function testCanConvertToJson() + { + $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag1, new EvalResult(0, 'value1', array())); + $state->addFlag($flag2, new EvalResult(1, 'value2', array())); + + $expected = array( + 'key1' => 'value1', + 'key2' => 'value2', + '$flagsState' => array( + 'key1' => array( + 'variation' => 0, + 'version' => 100, + 'trackEvents' => false + ), + 'key2' => array( + 'variation' => 1, + 'version' => 200, + 'trackEvents' => true, + 'debugEventsUntilDate' => 1000 + ) + ), + '$valid' => true + ); + $this->assertEquals($expected, $state->jsonSerialize()); + } + + public function testJsonEncodeUsesCustomSerializer() + { + $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag1, new EvalResult(0, 'value1', array())); + $state->addFlag($flag2, new EvalResult(1, 'value2', array())); + + $expected = $state->jsonSerialize(); + $json = json_encode($state); + $decoded = json_decode($json, true); + $this->assertEquals($expected, $decoded); + } +} diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php index 52d5493b2..66f24dddf 100644 --- a/tests/LDClientTest.php +++ b/tests/LDClientTest.php @@ -158,9 +158,10 @@ public function testAllFlagsStateReturnsState() 'trackEvents' => true, 'debugEventsUntilDate' => 1000 ) - ) + ), + '$valid' => true ); - $this->assertEquals($expectedState, $state->toJson()); + $this->assertEquals($expectedState, $state->jsonSerialize()); } public function testOnlyValidFeatureRequester() From 775f0a13255cca5fb9ea8f07c92f95e3851a1739 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 20 Aug 2018 21:02:53 -0700 Subject: [PATCH 07/17] indents --- tests/FeatureFlagsStateTest.php | 102 ++++++++++++++++---------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/tests/FeatureFlagsStateTest.php b/tests/FeatureFlagsStateTest.php index f61981909..9b97d0a4b 100644 --- a/tests/FeatureFlagsStateTest.php +++ b/tests/FeatureFlagsStateTest.php @@ -8,7 +8,7 @@ class FeatureFlagsStateTest extends \PHPUnit_Framework_TestCase { - private static $flag1Json = array( + private static $flag1Json = array( 'key' => 'key1', 'version' => 100, 'deleted' => false, @@ -38,74 +38,74 @@ class FeatureFlagsStateTest extends \PHPUnit_Framework_TestCase 'debugEventsUntilDate' => 1000 ); - public function testCanGetFlagValue() - { - $flag = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); - $state = new FeatureFlagsState(true); - $state->addFlag($flag, new EvalResult(0, 'value1', array())); + public function testCanGetFlagValue() + { + $flag = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag, new EvalResult(0, 'value1', array())); - $this->assertEquals('value1', $state->getFlagValue('key1')); - } + $this->assertEquals('value1', $state->getFlagValue('key1')); + } - public function testUnknownFlagReturnsNullValue() - { - $state = new FeatureFlagsState(true); - - $this->assertNull($state->getFlagValue('key1')); - } + public function testUnknownFlagReturnsNullValue() + { + $state = new FeatureFlagsState(true); + + $this->assertNull($state->getFlagValue('key1')); + } - public function testCanConvertToValuesMap() - { - $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); - $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); - $state = new FeatureFlagsState(true); - $state->addFlag($flag1, new EvalResult(0, 'value1', array())); - $state->addFlag($flag2, new EvalResult(0, 'value2', array())); + public function testCanConvertToValuesMap() + { + $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag1, new EvalResult(0, 'value1', array())); + $state->addFlag($flag2, new EvalResult(0, 'value2', array())); - $expected = array('key1' => 'value1', 'key2' => 'value2'); - $this->assertEquals($expected, $state->toValuesMap()); - } + $expected = array('key1' => 'value1', 'key2' => 'value2'); + $this->assertEquals($expected, $state->toValuesMap()); + } - public function testCanConvertToJson() - { - $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); - $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); - $state = new FeatureFlagsState(true); - $state->addFlag($flag1, new EvalResult(0, 'value1', array())); - $state->addFlag($flag2, new EvalResult(1, 'value2', array())); + public function testCanConvertToJson() + { + $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag1, new EvalResult(0, 'value1', array())); + $state->addFlag($flag2, new EvalResult(1, 'value2', array())); $expected = array( 'key1' => 'value1', 'key2' => 'value2', '$flagsState' => array( 'key1' => array( - 'variation' => 0, - 'version' => 100, - 'trackEvents' => false + 'variation' => 0, + 'version' => 100, + 'trackEvents' => false ), 'key2' => array( - 'variation' => 1, - 'version' => 200, - 'trackEvents' => true, - 'debugEventsUntilDate' => 1000 + 'variation' => 1, + 'version' => 200, + 'trackEvents' => true, + 'debugEventsUntilDate' => 1000 ) ), '$valid' => true ); $this->assertEquals($expected, $state->jsonSerialize()); - } + } - public function testJsonEncodeUsesCustomSerializer() - { - $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); - $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); - $state = new FeatureFlagsState(true); - $state->addFlag($flag1, new EvalResult(0, 'value1', array())); - $state->addFlag($flag2, new EvalResult(1, 'value2', array())); + public function testJsonEncodeUsesCustomSerializer() + { + $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag1, new EvalResult(0, 'value1', array())); + $state->addFlag($flag2, new EvalResult(1, 'value2', array())); - $expected = $state->jsonSerialize(); - $json = json_encode($state); - $decoded = json_decode($json, true); - $this->assertEquals($expected, $decoded); - } + $expected = $state->jsonSerialize(); + $json = json_encode($state); + $decoded = json_decode($json, true); + $this->assertEquals($expected, $decoded); + } } From 2f80b4c2c099480b6a125365e8cdb0a53a0889fc Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 21 Aug 2018 15:48:36 -0700 Subject: [PATCH 08/17] add ability to filter for client-side flags only --- src/LaunchDarkly/FeatureFlag.php | 18 ++++++++++++++++-- src/LaunchDarkly/LDClient.php | 9 ++++++++- tests/LDClientTest.php | 26 ++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/LaunchDarkly/FeatureFlag.php b/src/LaunchDarkly/FeatureFlag.php index f617cb672..893d855f0 100644 --- a/src/LaunchDarkly/FeatureFlag.php +++ b/src/LaunchDarkly/FeatureFlag.php @@ -31,6 +31,9 @@ class FeatureFlag protected $_trackEvents = false; /** @var int | null */ protected $_debugEventsUntilDate = null; + /** @var bool */ + protected $_clientSide = false; + // Note, trackEvents and debugEventsUntilDate are not used in EventProcessor, because // the PHP client doesn't do summary events. However, we need to capture them in case // they want to pass the flag data to the front end with allFlagsState(). @@ -47,7 +50,8 @@ protected function __construct($key, array $variations, $deleted, $trackEvents, - $debugEventsUntilDate) + $debugEventsUntilDate, + $clientSide) { $this->_key = $key; $this->_version = $version; @@ -62,6 +66,7 @@ protected function __construct($key, $this->_deleted = $deleted; $this->_trackEvents = $trackEvents; $this->_debugEventsUntilDate = $debugEventsUntilDate; + $this->_clientSide = $clientSide; } public static function getDecoder() @@ -80,7 +85,8 @@ public static function getDecoder() $v['variations'] ?: [], $v['deleted'], isset($v['trackEvents']) && $v['trackEvents'], - isset($v['debugEventsUntilDate']) ? $v['debugEventsUntilDate'] : null + isset($v['debugEventsUntilDate']) ? $v['debugEventsUntilDate'] : null, + isset($v['clientSide']) && $v['clientSide'] ); }; } @@ -252,4 +258,12 @@ public function getDebugEventsUntilDate() { return $this->_debugEventsUntilDate; } + + /** + * @return boolean + */ + public function isClientSide() + { + return $this->_clientSide; + } } diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index f528d25a3..25becc552 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -282,9 +282,12 @@ public function allFlags($user) * The most common use case for this method is to bootstrap a set of client-side feature flags from a back-end service. * To convert the state object into a JSON data structure, call its toJson() method. * @param $user LDUser the end user requesting the feature flags + * @param $options array optional properties affecting how the state is computed; set + * 'clientSideOnly' => true to specify that only flags marked for client-side use + * should be included (by default, all flags are included) * @return FeatureFlagsState a FeatureFlagsState object (will never be null; see FeatureFlagsState.isValid()) */ - public function allFlagsState($user) + public function allFlagsState($user, $options = array()) { if (is_null($user) || is_null($user->getKey())) { $this->_logger->warn("allFlagsState called with null user or null/empty user key! Returning empty state"); @@ -304,7 +307,11 @@ public function allFlagsState($user) } $state = new FeatureFlagsState(true); + $clientOnly = isset($options['clientSideOnly']) && $options['clientSideOnly']; foreach ($flags as $key => $flag) { + if ($clientOnly && !$flag->isClientSide()) { + continue; + } $result = $flag->evaluate($user, $this->_featureRequester); $state->addFlag($flag, $result); } diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php index 66f24dddf..d5f74f1c1 100644 --- a/tests/LDClientTest.php +++ b/tests/LDClientTest.php @@ -164,6 +164,32 @@ public function testAllFlagsStateReturnsState() $this->assertEquals($expectedState, $state->jsonSerialize()); } + public function testAllFlagsStateCanFilterForClientSideFlags() + { + $flag1Json = array('key' => 'server-side-1', 'on' => false, 'offVariation' => 0, 'variations' => array('a'), 'clientSide' => false); + $flag1 = FeatureFlag::decode($flag1Json); + $flag2Json = array('key' => 'server-side-2', 'on' => false, 'offVariation' => 0, 'variations' => array('b'), 'clientSide' => false); + $flag2 = FeatureFlag::decode($flag2Json); + $flag3Json = array('key' => 'client-side-1', 'on' => false, 'offVariation' => 0, 'variations' => array('value1'), 'clientSide' => true); + $flag3 = FeatureFlag::decode($flag3Json); + $flag4Json = array('key' => 'client-side-2', 'on' => false, 'offVariation' => 0, 'variations' => array('value2'), 'clientSide' => true); + $flag4 = FeatureFlag::decode($flag4Json); + MockFeatureRequester::$flags = array( + $flag1->getKey() => $flag1, $flag2->getKey() => $flag2, $flag3->getKey() => $flag3, $flag4->getKey() => $flag4 + ); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $builder = new LDUserBuilder(3); + $user = $builder->build(); + $state = $client->allFlagsState($user, array('clientSideOnly' => true)); + + $this->assertTrue($state->isValid()); + $this->assertEquals(array('client-side-1' => 'value1', 'client-side-2' => 'value2'), $state->toValuesMap()); + } + public function testOnlyValidFeatureRequester() { $this->setExpectedException(InvalidArgumentException::class); From fd08375e6330a0b32785f21f1c62101acd9d37dc Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 21 Aug 2018 15:58:11 -0700 Subject: [PATCH 09/17] fix test to fill in all required flag fields --- tests/LDClientTest.php | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php index d5f74f1c1..e523bd3e2 100644 --- a/tests/LDClientTest.php +++ b/tests/LDClientTest.php @@ -166,14 +166,19 @@ public function testAllFlagsStateReturnsState() public function testAllFlagsStateCanFilterForClientSideFlags() { - $flag1Json = array('key' => 'server-side-1', 'on' => false, 'offVariation' => 0, 'variations' => array('a'), 'clientSide' => false); - $flag1 = FeatureFlag::decode($flag1Json); - $flag2Json = array('key' => 'server-side-2', 'on' => false, 'offVariation' => 0, 'variations' => array('b'), 'clientSide' => false); - $flag2 = FeatureFlag::decode($flag2Json); - $flag3Json = array('key' => 'client-side-1', 'on' => false, 'offVariation' => 0, 'variations' => array('value1'), 'clientSide' => true); - $flag3 = FeatureFlag::decode($flag3Json); - $flag4Json = array('key' => 'client-side-2', 'on' => false, 'offVariation' => 0, 'variations' => array('value2'), 'clientSide' => true); - $flag4 = FeatureFlag::decode($flag4Json); + $flagJson = array('key' => 'server-side-1', 'version' => 1, 'on' => false, 'salt' => '', 'deleted' => false, + 'targets' => array(), 'rules' => array(), 'prerequisites' => array(), 'fallthrough' => array(), + 'offVariation' => 0, 'variations' => array('a'), 'clientSide' => false); + $flag1 = FeatureFlag::decode($flagJson); + $flagJson['key'] = 'server-side-2'; + $flag2 = FeatureFlag::decode($flagJson); + $flagJson['key'] = 'client-side-1'; + $flagJson['clientSide'] = true; + $flagJson['variations'] = array('value1'); + $flag3 = FeatureFlag::decode($flagJson); + $flagJson['key'] = 'client-side-2'; + $flagJson['variations'] = array('value2'); + $flag4 = FeatureFlag::decode($flagJson); MockFeatureRequester::$flags = array( $flag1->getKey() => $flag1, $flag2->getKey() => $flag2, $flag3->getKey() => $flag3, $flag4->getKey() => $flag4 ); From 2e9829c6656a2746bccf4606456f6920bdce01a5 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 16:49:28 -0700 Subject: [PATCH 10/17] implement evaluation with explanations --- src/LaunchDarkly/EvalResult.php | 25 +-- src/LaunchDarkly/EvaluationDetail.php | 68 ++++++++ src/LaunchDarkly/EvaluationReason.php | 222 +++++++++++++++++++++++++ src/LaunchDarkly/FeatureFlag.php | 165 ++++++++++-------- src/LaunchDarkly/FeatureFlagsState.php | 39 ++++- src/LaunchDarkly/LDClient.php | 105 +++++++----- src/LaunchDarkly/Rule.php | 14 +- src/LaunchDarkly/Util.php | 5 +- tests/EvaluationReasonTest.php | 57 +++++++ tests/FeatureFlagTest.php | 203 +++++++++++++++++----- tests/FeatureFlagsStateTest.php | 42 ++++- tests/LDClientTest.php | 193 +++++++++++++++++++-- 12 files changed, 937 insertions(+), 201 deletions(-) create mode 100644 src/LaunchDarkly/EvaluationDetail.php create mode 100644 src/LaunchDarkly/EvaluationReason.php create mode 100644 tests/EvaluationReasonTest.php diff --git a/src/LaunchDarkly/EvalResult.php b/src/LaunchDarkly/EvalResult.php index f692abac1..6b4b4ab21 100644 --- a/src/LaunchDarkly/EvalResult.php +++ b/src/LaunchDarkly/EvalResult.php @@ -4,37 +4,28 @@ class EvalResult { - private $_variation = null; - private $_value = null; + /** @var EvaluationDetail */ + private $_detail = null; /** @var array */ private $_prerequisiteEvents = []; /** * EvalResult constructor. - * @param null $value + * @param EvaluationDetail $detail * @param array $prerequisiteEvents */ - public function __construct($variation, $value, array $prerequisiteEvents) + public function __construct($detail, array $prerequisiteEvents) { - $this->_variation = $variation; - $this->_value = $value; + $this->_detail = $detail; $this->_prerequisiteEvents = $prerequisiteEvents; } /** - * @return int | null + * @return EvaluationDetail */ - public function getVariation() + public function getDetail() { - return $this->_variation; - } - - /** - * @return null - */ - public function getValue() - { - return $this->_value; + return $this->_detail; } /** diff --git a/src/LaunchDarkly/EvaluationDetail.php b/src/LaunchDarkly/EvaluationDetail.php new file mode 100644 index 000000000..7983ba1a1 --- /dev/null +++ b/src/LaunchDarkly/EvaluationDetail.php @@ -0,0 +1,68 @@ +_value = $value; + $this->_variationIndex = $variationIndex; + $this->_reason = $reason; + } + + /** + * Returns the value of the flag variation for the user. + * + * @return mixed + */ + public function getValue() + { + return $this->_value; + } + + /** + * Returns the index of the flag variation for the user, e.g. 0 for the first variation - + * or null if it was the default value. + * + * @return int | null + */ + public function getVariationIndex() + { + return $this->_variationIndex; + } + + /** + * Returns information about how the flag value was calculated. + * + * @return EvaluationReason + */ + public function getReason() + { + return $this->_reason; + } + + /** + * Returns true if the flag evaluated to the default value, rather than one of its variations. + * + * @return bool + */ + public function isDefaultValue() + { + return ($this->_variationIndex === null); + } +} diff --git a/src/LaunchDarkly/EvaluationReason.php b/src/LaunchDarkly/EvaluationReason.php new file mode 100644 index 000000000..92e2b3403 --- /dev/null +++ b/src/LaunchDarkly/EvaluationReason.php @@ -0,0 +1,222 @@ +_kind = $kind; + $this->_errorKind = $errorKind; + $this->_ruleIndex = $ruleIndex; + $this->_ruleId = $ruleId; + $this->_prerequisiteKey = $prerequisiteKey; + } + + /** + * Returns a constant indicating the general category of the reason, such as OFF. + * @return string + */ + public function getKind() + { + return $this->_kind; + } + + /** + * Returns a constant indicating the nature of the error, if getKind() is OFF. Otherwise + * returns null. + * @return string|null + */ + public function getErrorKind() + { + return $this->_errorKind; + } + + /** + * Returns the positional index of the rule that was matched (0 for the first), if getKind() + * is RULE_MATCH. Otherwise returns null. + * @return int|null + */ + public function getRuleIndex() + { + return $this->_ruleIndex; + } + + /** + * Returns the unique identifier of the rule that was matched, if getKind() is RULE_MATCH. + * Otherwise returns null. + * @return string|null + */ + public function getRuleId() + { + return $this->_ruleId; + } + + /** + * Returns the key of the prerequisite feature flag that failed, if getKind() is + * PREREQUISITE_FAILED. Otherwise returns null. + * @return string|null + */ + public function getPrerequisiteKey() + { + return $this->_prerequisiteKey; + } + + /** + * Returns a simple string representation of this object. + */ + public function __toString() + { + switch ($this->_kind) { + case self::RULE_MATCH: + return $this->_kind . '(' . $this->_ruleIndex . ',' . $this->_ruleId . ')'; + case self::PREREQUISITE_FAILED: + return $this->_kind . '(' . $this->_prerequisiteKey . ')'; + case self::ERROR: + return $this->_kind . '(' . $this->_errorKind . ')'; + default: + return $this->_kind; + } + } + + /** + * Returns a JSON representation of this object. This method is used automatically + * if you call json_encode(). + */ + public function jsonSerialize() + { + $ret = array('kind' => $this->_kind); + if ($this->_errorKind !== null) { + $ret['errorKind'] = $this->_errorKind; + } + if ($this->_ruleIndex !== null) { + $ret['ruleIndex'] = $this->_ruleIndex; + } + if ($this->_ruleId !== null) { + $ret['ruleId'] = $this->_ruleId; + } + if ($this->_prerequisiteKey !== null) { + $ret['prerequisiteKey'] = $this->_prerequisiteKey; + } + return $ret; + } +} diff --git a/src/LaunchDarkly/FeatureFlag.php b/src/LaunchDarkly/FeatureFlag.php index 893d855f0..1ea5af29a 100644 --- a/src/LaunchDarkly/FeatureFlag.php +++ b/src/LaunchDarkly/FeatureFlag.php @@ -102,121 +102,140 @@ public function isOn() } /** - * @param $user LDUser - * @param $featureRequester FeatureRequester - * @return EvalResult|null + * @param LDUser $user + * @param FeatureRequester $featureRequester + * @param bool $includeReasonsInEvents + * @return EvalResult */ - public function evaluate($user, $featureRequester) + public function evaluate($user, $featureRequester, $includeReasonsInEvents = false) { - $prereqEvents = array(); - if (is_null($user) || is_null($user->getKey())) { - return new EvalResult(null, $prereqEvents); - } if ($this->isOn()) { - $result = $this->_evaluate($user, $featureRequester, $prereqEvents); - if ($result !== null) { - return $result; + $prereqEvents = array(); + $detail = $this->evaluateInternal($user, $featureRequester, $prereqEvents, $includeReasonsInEvents); + return new EvalResult($detail, $prereqEvents); + } + return new EvalResult($this->getOffValue(EvaluationReason::off()), array()); + } + + /** + * @param LDUser $user + * @param FeatureRequester $featureRequester + * @param array $events + * @param bool $includeReasonsInEvents + * @return EvaluationDetail + */ + private function evaluateInternal($user, $featureRequester, &$events, $includeReasonsInEvents) + { + $prereqFailureReason = $this->checkPrerequisites($user, $featureRequester, $events, $includeReasonsInEvents); + if ($prereqFailureReason !== null) { + return $this->getOffValue($prereqFailureReason); + } + + // Check to see if targets match + if ($this->_targets != null) { + foreach ($this->_targets as $target) { + foreach ($target->getValues() as $value) { + if ($value === $user->getKey()) { + return $this->getVariation($target->getVariation(), EvaluationReason::targetMatch()); + } + } + } + } + // Now walk through the rules and see if any match + if ($this->_rules != null) { + foreach ($this->_rules as $i => $rule) { + if ($rule->matchesUser($user, $featureRequester)) { + return $this->getValueForVariationOrRollout($rule, $user, + EvaluationReason::ruleMatch($i, $rule->getId())); + } } } - $offVariationValue = $this->getOffVariationValue(); - return new EvalResult($this->_offVariation, $offVariationValue, $prereqEvents); + return $this->getValueForVariationOrRollout($this->_fallthrough, $user, EvaluationReason::fallthrough()); } /** - * @param $user LDUser - * @param $featureRequester FeatureRequester - * @param $events - * @return EvalResult|null + * @param LDUser $user + * @param FeatureRequester $featureRequester + * @param array $events + * @param bool $includeReasonsInEvents + * @return EvaluationReason|null */ - private function _evaluate($user, $featureRequester, &$events) + private function checkPrerequisites($user, $featureRequester, &$events, $includeReasonsInEvents) { - $prereqOk = true; if ($this->_prerequisites != null) { foreach ($this->_prerequisites as $prereq) { + $prereqOk = true; try { $prereqEvalResult = null; $prereqFeatureFlag = $featureRequester->getFeature($prereq->getKey()); if ($prereqFeatureFlag == null) { - return null; + $prereqOk = false; } elseif ($prereqFeatureFlag->isOn()) { - $prereqEvalResult = $prereqFeatureFlag->_evaluate($user, $featureRequester, $events); + $prereqEvalResult = $prereqFeatureFlag->evaluateInternal($user, $featureRequester, $events, $includeReasonsInEvents); $variation = $prereq->getVariation(); - if ($prereqEvalResult === null || $variation === null || $prereqEvalResult->getVariation() !== $variation) { + if ($prereqEvalResult->getVariationIndex() !== $variation) { $prereqOk = false; } } else { $prereqOk = false; } - array_push($events, Util::newFeatureRequestEvent($prereqFeatureFlag->getKey(), $user, - $prereqEvalResult === null ? null : $prereqEvalResult->getVariation(), - $prereqEvalResult === null ? null : $prereqEvalResult->getValue(), - null, $prereqFeatureFlag->getVersion(), $this->_key)); + if ($prereqFeatureFlag) { + array_push($events, Util::newFeatureRequestEvent($prereq->getKey(), $user, + $prereqEvalResult ? $prereqEvalResult->getVariationIndex() : null, + $prereqEvalResult ? $prereqEvalResult->getValue() : null, + null, $prereqFeatureFlag->getVersion(), $this->_key, + ($includeReasonsInEvents && $prereqEvalResult) ? $prereqEvalResult->getReason() : null + )); + } } catch (EvaluationException $e) { $prereqOk = false; } + if (!$prereqOk) { + return EvaluationReason::prerequisiteFailed($prereq->getKey()); + } } } - if ($prereqOk) { - $variation = $this->evaluateIndex($user, $featureRequester); - $value = $this->getVariation($variation); - return new EvalResult($variation, $value, $events); - } return null; } /** - * @param $user LDUser - * @return int|null + * @param int $index + * @param EvaluationReason $reason + * @return EvaluationDetail */ - private function evaluateIndex($user, $featureRequester) + private function getVariation($index, $reason) { - // Check to see if targets match - if ($this->_targets != null) { - foreach ($this->_targets as $target) { - foreach ($target->getValues() as $value) { - if ($value === $user->getKey()) { - return $target->getVariation(); - } - } - } - } - // Now walk through the rules and see if any match - if ($this->_rules != null) { - foreach ($this->_rules as $rule) { - if ($rule->matchesUser($user, $featureRequester)) { - return $rule->variationIndexForUser($user, $this->_key, $this->_salt); - } - } + if ($index < 0 || $index >= count($this->_variations)) { + return new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)); } - // Walk through the fallthrough and see if it matches - return $this->_fallthrough->variationIndexForUser($user, $this->_key, $this->_salt); + return new EvaluationDetail($this->_variations[$index], $index, $reason); } - private function getVariation($index) + /** + * @param EvaluationReason reason + * @return EvaluationDetail + */ + private function getOffValue($reason) { - // If the supplied index is null, then rules didn't match, and we want to return - // the off variation - if (!isset($index)) { - return null; - } - // If the index doesn't refer to a valid variation, that's an unexpected exception and we will - // return the default variation - if ($index >= count($this->_variations)) { - throw new EvaluationException("Invalid Index"); - } else { - return $this->_variations[$index]; + if ($this->_offVariation === null) { + return new EvaluationDetail(null, null, $reason); } + return $this->getVariation($this->_offVariation, $reason); } - - public function getOffVariationValue() + + /** + * @param VariationOrRollout $r + * @param LDUser $user + * @param EvaluationReason $reason + * @return EvaluationDetail + */ + private function getValueForVariationOrRollout($r, $user, $reason) { - if ($this->_offVariation === null) { - return null; - } - if ($this->_offVariation >= count($this->_variations)) { - throw new EvaluationException("Invalid offVariation index"); + $index = $r->variationIndexForUser($user, $this->_key, $this->_salt); + if ($index === null) { + return new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)); } - return $this->_variations[$this->_offVariation]; + return $this->getVariation($index, $reason); } /** diff --git a/src/LaunchDarkly/FeatureFlagsState.php b/src/LaunchDarkly/FeatureFlagsState.php index 87c84d2b4..3c11338ef 100644 --- a/src/LaunchDarkly/FeatureFlagsState.php +++ b/src/LaunchDarkly/FeatureFlagsState.php @@ -28,15 +28,18 @@ public function __construct($valid, $flagValues = array(), $flagMetadata = array /** * Used internally to build the state map. */ - public function addFlag($flag, $evalResult) + public function addFlag($flag, $detail, $withReason = false) { - $this->_flagValues[$flag->getKey()] = $evalResult->getValue(); + $this->_flagValues[$flag->getKey()] = $detail->getValue(); $meta = array(); - if (!is_null($evalResult->getVariation())) { - $meta['variation'] = $evalResult->getVariation(); + if (!is_null($detail->getVariationIndex())) { + $meta['variation'] = $detail->getVariationIndex(); } $meta['version'] = $flag->getVersion(); $meta['trackEvents'] = $flag->isTrackEvents(); + if ($withReason) { + $meta['reason'] = $detail->getReason(); + } if ($flag->getDebugEventsUntilDate()) { $meta['debugEventsUntilDate'] = $flag->getDebugEventsUntilDate(); } @@ -55,7 +58,7 @@ public function isValid() /** * Returns the value of an individual feature flag at the time the state was recorded. - * @param $key string + * @param $key string the feature flag key * @return mixed the flag's value; null if the flag returned the default value, or if there was no such flag */ public function getFlagValue($key) @@ -63,6 +66,22 @@ public function getFlagValue($key) return isset($this->_flagValues[$key]) ? $this->_flagValues[$key] : null; } + /** + * Returns the evaluation reason for an individual feature flag (as returned by LDClient.variationDetail()) + * at the time the state was recorded. + * @param $key string the feature flag key + * @return EvaluationReason|null the evaluation reason; null if reasons were not recorded, or if there + * was no such flag + */ + public function getFlagReason($key) + { + if (isset($this->_flagMetadata[$key])) { + $meta = $this->_flagMetadata[$key]; + return isset($meta['reason']) ? $meta['reason'] : null; + } + return null; + } + /** * Returns an associative array of flag keys to flag values. If a flag would have evaluated to the default * value, its value will be null. @@ -88,7 +107,15 @@ public function toValuesMap() public function jsonSerialize() { $ret = array_replace([], $this->_flagValues); - $ret['$flagsState'] = $this->_flagMetadata; + $metaMap = array(); + foreach ($this->_flagMetadata as $key => $meta) { + $meta = array_replace([], $meta); + if (isset($meta['reason'])) { + $meta['reason'] = $meta['reason']->jsonSerialize(); + } + $metaMap[$key] = $meta; + } + $ret['$flagsState'] = $metaMap; $ret['$valid'] = $this->_valid; return $ret; } diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index 25becc552..7ff437e21 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -134,10 +134,51 @@ private function getFeatureRequester($sdkKey, array $options) */ public function variation($key, $user, $default = false) { + $detail = $this->variationDetailInternal($key, $user, $default, false); + return $detail->getValue(); + } + + /** + * Calculates the value of a feature flag, and returns an object that describes the way the + * value was determined. The "reason" property in the result will also be included in + * analytics events, if you are capturing detailed event data for this flag. + * + * @param string $key The unique key for the feature flag + * @param LDUser $user The end user requesting the flag + * @param mixed $default The default value of the flag + * + * @return EvaluationDetail An EvaluationDetail object that includes the feature flag value + * and evaluation reason. + */ + public function variationDetail($key, $user, $default = false) + { + return $this->variationDetailInternal($key, $user, $default, true); + } + + /** + * @param string $key + * @param LDUser $user + * @param mixed $default + * @param bool $includeReasonsInEvents + */ + private function variationDetailInternal($key, $user, $default, $includeReasonsInEvents) { $default = $this->_get_default($key, $default); + $errorResult = function($errorKind) use ($key, $default) { + return new EvaluationDetail($default, null, EvaluationReason::error($errorKind)); + }; + $sendEvent = function($detail, $flag) use ($key, $user, $default, $includeReasonsInEvents) { + if ($this->isOffline() || !$this->_send_events) { + return; + } + $event = Util::newFeatureRequestEvent($key, $user, $detail->getVariationIndex(), $detail->getValue(), + $default, $flag ? $flag->getVersion() : null, null, + $includeReasonsInEvents ? $detail->getReason() : null); + $this->_eventProcessor->enqueue($event); + }; + if ($this->_offline) { - return $default; + return $errorResult(EvaluationReason::CLIENT_NOT_READY_ERROR); } try { @@ -148,43 +189,44 @@ public function variation($key, $user, $default = false) $flag = $this->_featureRequester->getFeature($key); } catch (UnrecoverableHTTPStatusException $e) { $this->handleUnrecoverableError(); - return $default; + return $errorResult(EvaluationReason::EXCEPTION_ERROR); } if (is_null($flag)) { - $this->_sendFlagRequestEvent($key, $user, null, $default, $default); - return $default; + $result = $errorResult(EvaluationReason::FLAG_NOT_FOUND_ERROR); + $sendEvent($result, null); + return $result; } if (is_null($user) || is_null($user->getKey())) { - $this->_sendFlagRequestEvent($key, $user, null, $default, $default, $flag->getVersion()); + $result = $errorResult(EvaluationReason::USER_NOT_SPECIFIED_ERROR); + $sendEvent($result, $flag); $this->_logger->warning("Variation called with null user or null user key! Returning default value"); - return $default; + return $result; } - $evalResult = $flag->evaluate($user, $this->_featureRequester); + $evalResult = $flag->evaluate($user, $this->_featureRequester, $includeReasonsInEvents); if (!$this->isOffline() && $this->_send_events) { foreach ($evalResult->getPrerequisiteEvents() as $e) { $this->_eventProcessor->enqueue($e); } } - if ($evalResult !== null && $evalResult->getValue() !== null) { - $this->_sendFlagRequestEvent($key, $user, $evalResult->getVariation(), $evalResult->getValue(), $default, $flag->getVersion()); - return $evalResult->getValue(); - } else { - $this->_sendFlagRequestEvent($key, $user, null, $default, $default, $flag->getVersion()); - return $default; + $detail = $evalResult->getDetail(); + if ($detail->isDefaultValue()) { + $detail = new EvaluationDetail($default, null, $detail->getReason()); } + $sendEvent($detail, $flag); + return $detail; } catch (\Exception $e) { $this->_logger->error("Caught $e"); + $result = $errorResult(EvaluationReason::EXCEPTION_ERROR); + try { + $sendEvent($result, null); + } catch (\Exception $e) { + $this->_logger->error("Caught $e"); + } + return $result; } - try { - $this->_sendFlagRequestEvent($key, $user, null, $default, $default); - } catch (\Exception $e) { - $this->_logger->error("Caught $e"); - } - return $default; } - /** @deprecated Use variation() instead. * @param $key * @param $user @@ -284,7 +326,8 @@ public function allFlags($user) * @param $user LDUser the end user requesting the feature flags * @param $options array optional properties affecting how the state is computed; set * 'clientSideOnly' => true to specify that only flags marked for client-side use - * should be included (by default, all flags are included) + * should be included (by default, all flags are included); set 'withReasons' => true + * to include evaluation reasons (see variationDetail) * @return FeatureFlagsState a FeatureFlagsState object (will never be null; see FeatureFlagsState.isValid()) */ public function allFlagsState($user, $options = array()) @@ -308,12 +351,13 @@ public function allFlagsState($user, $options = array()) $state = new FeatureFlagsState(true); $clientOnly = isset($options['clientSideOnly']) && $options['clientSideOnly']; + $withReasons = isset($options['withReasons']) && $options['withReasons']; foreach ($flags as $key => $flag) { if ($clientOnly && !$flag->isClientSide()) { continue; } $result = $flag->evaluate($user, $this->_featureRequester); - $state->addFlag($flag, $result); + $state->addFlag($flag, $result->getDetail(), $withReasons); } return $state; } @@ -343,23 +387,6 @@ public function flush() } } - /** - * @param $key string - * @param $user LDUser - * @param $variation int | null - * @param $value mixed - * @param $default - * @param $version int | null - * @param string | null $prereqOf - */ - protected function _sendFlagRequestEvent($key, $user, $variation, $value, $default, $version = null, $prereqOf = null) - { - if ($this->isOffline() || !$this->_send_events) { - return; - } - $this->_eventProcessor->enqueue(Util::newFeatureRequestEvent($key, $user, $variation, $value, $default, $version, $prereqOf)); - } - protected function _get_default($key, $default) { if (array_key_exists($key, $this->_defaults)) { diff --git a/src/LaunchDarkly/Rule.php b/src/LaunchDarkly/Rule.php index daf9c6e11..39d1a9622 100644 --- a/src/LaunchDarkly/Rule.php +++ b/src/LaunchDarkly/Rule.php @@ -4,12 +4,15 @@ class Rule extends VariationOrRollout { + /** @var string */ + private $_id = null; /** @var Clause[] */ private $_clauses = array(); - protected function __construct($variation, $rollout, array $clauses) + protected function __construct($variation, $rollout, $id, array $clauses) { parent::__construct($variation, $rollout); + $this->_id = $id; $this->_clauses = $clauses; } @@ -19,6 +22,7 @@ public static function getDecoder() return new Rule( isset($v['variation']) ? $v['variation'] : null, isset($v['rollout']) ? call_user_func(Rollout::getDecoder(), $v['rollout']) : null, + isset($v['id']) ? $v['id'] : null, array_map(Clause::getDecoder(), $v['clauses'])); }; } @@ -37,6 +41,14 @@ public function matchesUser($user, $featureRequester) return true; } + /** + * @return string + */ + public function getId() + { + return $this->_id; + } + /** * @return Clause[] */ diff --git a/src/LaunchDarkly/Util.php b/src/LaunchDarkly/Util.php index 1fbf79010..e9afd00af 100644 --- a/src/LaunchDarkly/Util.php +++ b/src/LaunchDarkly/Util.php @@ -37,7 +37,7 @@ public static function currentTimeUnixMillis() * @param null $prereqOf string | null * @return array */ - public static function newFeatureRequestEvent($key, $user, $variation, $value, $default, $version = null, $prereqOf = null) + public static function newFeatureRequestEvent($key, $user, $variation, $value, $default, $version = null, $prereqOf = null, $reason = null) { $event = array(); $event['user'] = $user; @@ -49,6 +49,9 @@ public static function newFeatureRequestEvent($key, $user, $variation, $value, $ $event['default'] = $default; $event['version'] = $version; $event['prereqOf'] = $prereqOf; + if ($reason !== null) { + $event['reason'] = $reason->jsonSerialize(); + } return $event; } diff --git a/tests/EvaluationReasonTest.php b/tests/EvaluationReasonTest.php new file mode 100644 index 000000000..759dc3ece --- /dev/null +++ b/tests/EvaluationReasonTest.php @@ -0,0 +1,57 @@ +assertEquals(array('kind' => 'OFF'), json_decode($json, true)); + $this->assertEquals('OFF', (string)$reason); + } + + public function testFallthroughReasonSerialization() + { + $reason = EvaluationReason::fallthrough(); + $json = json_encode($reason); + $this->assertEquals(array('kind' => 'FALLTHROUGH'), json_decode($json, true)); + $this->assertEquals('FALLTHROUGH', (string)$reason); + } + + public function testTargetMatchReasonSerialization() + { + $reason = EvaluationReason::targetMatch(); + $json = json_encode($reason); + $this->assertEquals(array('kind' => 'TARGET_MATCH'), json_decode($json, true)); + $this->assertEquals('TARGET_MATCH', (string)$reason); + } + + public function testRuleMatchReasonSerialization() + { + $reason = EvaluationReason::ruleMatch(0, 'id'); + $json = json_encode($reason); + $this->assertEquals(array('kind' => 'RULE_MATCH', 'ruleIndex' => 0, 'ruleId' => 'id'), + json_decode($json, true)); + $this->assertEquals('RULE_MATCH(0,id)', (string)$reason); + } + + public function testPrerequisiteFailedReasonSerialization() + { + $reason = EvaluationReason::prerequisiteFailed('key'); + $json = json_encode($reason); + $this->assertEquals(array('kind' => 'PREREQUISITE_FAILED', 'prerequisiteKey' => 'key'), + json_decode($json, true)); + $this->assertEquals('PREREQUISITE_FAILED(key)', (string)$reason); + } + + public function testErrorReasonSerialization() + { + $reason = EvaluationReason::error(EvaluationReason::EXCEPTION_ERROR); + $json = json_encode($reason); + $this->assertEquals(array('kind' => 'ERROR', 'errorKind' => 'EXCEPTION'), json_decode($json, true)); + $this->assertEquals('ERROR(EXCEPTION)', (string)$reason); + } +} diff --git a/tests/FeatureFlagTest.php b/tests/FeatureFlagTest.php index e73531b76..2ea5f9e03 100644 --- a/tests/FeatureFlagTest.php +++ b/tests/FeatureFlagTest.php @@ -1,7 +1,10 @@ '' ); $flag = FeatureFlag::decode($flagJson); - $ub = new LDUserBuilder('x'); - $user = $ub->build(); - $result = $flag->evaluate($user, null); - self::assertEquals(1, $result->getVariation()); - self::assertEquals('off', $result->getValue()); + $result = $flag->evaluate(new LDUser('user'), null); + $detail = new EvaluationDetail('off', 1, EvaluationReason::off()); + self::assertEquals($detail, $result->getDetail()); self::assertEquals(array(), $result->getPrerequisiteEvents()); } @@ -232,12 +233,56 @@ public function testFlagReturnsNullIfFlagIsOffAndOffVariationIsUnspecified() 'salt' => '' ); $flag = FeatureFlag::decode($flagJson); - $ub = new LDUserBuilder('x'); - $user = $ub->build(); - $result = $flag->evaluate($user, null); - self::assertNull($result->getVariation()); - self::assertNull($result->getValue()); + $result = $flag->evaluate(new LDUser('user'), null); + $detail = new EvaluationDetail(null, null, EvaluationReason::off()); + self::assertEquals($detail, $result->getDetail()); + self::assertEquals(array(), $result->getPrerequisiteEvents()); + } + + public function testFlagReturnsErrorIfOffVariationIsTooHigh() + { + $flagJson = array( + 'key' => 'feature', + 'version' => 1, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => 999, + 'fallthrough' => array('variation' => 0), + 'variations' => array('fall', 'off', 'on'), + 'salt' => '' + ); + $flag = FeatureFlag::decode($flagJson); + + $result = $flag->evaluate(new LDUser('user'), null); + $detail = new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)); + self::assertEquals($detail, $result->getDetail()); + self::assertEquals(array(), $result->getPrerequisiteEvents()); + } + + public function testFlagReturnsErrorIfOffVariationIsNegative() + { + $flagJson = array( + 'key' => 'feature', + 'version' => 1, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => -1, + 'fallthrough' => array('variation' => 0), + 'variations' => array('fall', 'off', 'on'), + 'salt' => '' + ); + $flag = FeatureFlag::decode($flagJson); + + $result = $flag->evaluate(new LDUser('user'), null); + $detail = new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)); + self::assertEquals($detail, $result->getDetail()); self::assertEquals(array(), $result->getPrerequisiteEvents()); } @@ -264,8 +309,8 @@ public function testFlagReturnsOffVariationIfPrerequisiteIsNotFound() $requester = new MockFeatureRequesterForFeature(); $result = $flag->evaluate($user, $requester); - self::assertEquals(1, $result->getVariation()); - self::assertEquals('off', $result->getValue()); + $detail = new EvaluationDetail('off', 1, EvaluationReason::prerequisiteFailed('feature1')); + self::assertEquals($detail, $result->getDetail()); self::assertEquals(array(), $result->getPrerequisiteEvents()); } @@ -308,8 +353,8 @@ public function testFlagReturnsOffVariationAndEventIfPrerequisiteIsNotMet() $requester->val = $flag1; $result = $flag0->evaluate($user, $requester); - self::assertEquals(1, $result->getVariation()); - self::assertEquals('off', $result->getValue()); + $detail = new EvaluationDetail('off', 1, EvaluationReason::prerequisiteFailed('feature1')); + self::assertEquals($detail, $result->getDetail()); $events = $result->getPrerequisiteEvents(); self::assertEquals(1, count($events)); @@ -360,8 +405,8 @@ public function testFlagReturnsFallthroughVariationAndEventIfPrerequisiteIsMetAn $requester->val = $flag1; $result = $flag0->evaluate($user, $requester); - self::assertEquals(0, $result->getVariation()); - self::assertEquals('fall', $result->getValue()); + $detail = new EvaluationDetail('fall', 0, EvaluationReason::fallthrough()); + self::assertEquals($detail, $result->getDetail()); $events = $result->getPrerequisiteEvents(); self::assertEquals(1, count($events)); @@ -395,12 +440,12 @@ public function testFlagMatchesUserFromTargets() $user = $ub->build(); $result = $flag->evaluate($user, null); - self::assertEquals(2, $result->getVariation()); - self::assertEquals('on', $result->getValue()); + $detail = new EvaluationDetail('on', 2, EvaluationReason::targetMatch()); + self::assertEquals($detail, $result->getDetail()); self::assertEquals(array(), $result->getPrerequisiteEvents()); } - public function testFlagMatchesUserFromRules() + private function makeBooleanFlagWithRules(array $rules) { $flagJson = array( 'key' => 'feature', @@ -409,31 +454,111 @@ public function testFlagMatchesUserFromRules() 'on' => true, 'targets' => array(), 'prerequisites' => array(), - 'rules' => array( - array( - 'clauses' => array( - array( - 'attribute' => 'key', - 'op' => 'in', - 'values' => array('userkey'), - 'negate' => false - ) - ), - 'variation' => 2 - ) - ), - 'offVariation' => 1, + 'rules' => $rules, + 'offVariation' => 0, 'fallthrough' => array('variation' => 0), - 'variations' => array('fall', 'off', 'on'), + 'variations' => array(false, true), 'salt' => '' ); - $flag = FeatureFlag::decode($flagJson); + return FeatureFlag::decode($flagJson); + } + + public function testFlagMatchesUserFromRules() + { + $flag = $this->makeBooleanFlagWithRules(array( + array( + 'id' => 'ruleid', + 'clauses' => array( + array('attribute' => 'key', 'op' => 'in', 'values' => array('userkey'), 'negate' => false) + ), + 'variation' => 1 + ) + )); + $ub = new LDUserBuilder('userkey'); + $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 testFlagReturnsErrorIfRuleVariationIsTooHigh() + { + $flag = $this->makeBooleanFlagWithRules(array( + array( + 'id' => 'ruleid', + 'clauses' => array( + array('attribute' => 'key', 'op' => 'in', 'values' => array('userkey'), 'negate' => false) + ), + 'variation' => 999 + ) + )); + $ub = new LDUserBuilder('userkey'); + $user = $ub->build(); + + $result = $flag->evaluate($user, null); + $detail = new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)); + self::assertEquals($detail, $result->getDetail()); + self::assertEquals(array(), $result->getPrerequisiteEvents()); + } + + public function testFlagReturnsErrorIfRuleVariationIsNegative() + { + $flag = $this->makeBooleanFlagWithRules(array( + array( + 'id' => 'ruleid', + 'clauses' => array( + array('attribute' => 'key', 'op' => 'in', 'values' => array('userkey'), 'negate' => false) + ), + 'variation' => -1 + ) + )); + $ub = new LDUserBuilder('userkey'); + $user = $ub->build(); + + $result = $flag->evaluate($user, null); + $detail = new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)); + self::assertEquals($detail, $result->getDetail()); + self::assertEquals(array(), $result->getPrerequisiteEvents()); + } + + public function testFlagReturnsErrorIfRuleHasNoVariationOrRollout() + { + $flag = $this->makeBooleanFlagWithRules(array( + array( + 'id' => 'ruleid', + 'clauses' => array( + array('attribute' => 'key', 'op' => 'in', 'values' => array('userkey'), 'negate' => false) + ) + ) + )); + $ub = new LDUserBuilder('userkey'); + $user = $ub->build(); + + $result = $flag->evaluate($user, null); + $detail = new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)); + self::assertEquals($detail, $result->getDetail()); + self::assertEquals(array(), $result->getPrerequisiteEvents()); + } + + public function testFlagReturnsErrorIfRuleHasRolloutWithNoVariations() + { + $flag = $this->makeBooleanFlagWithRules(array( + array( + 'id' => 'ruleid', + 'clauses' => array( + array('attribute' => 'key', 'op' => 'in', 'values' => array('userkey'), 'negate' => false) + ), + 'rollout' => array('variations' => array()) + ) + )); $ub = new LDUserBuilder('userkey'); $user = $ub->build(); $result = $flag->evaluate($user, null); - self::assertEquals(2, $result->getVariation()); - self::assertEquals('on', $result->getValue()); + $detail = new EvaluationDetail(null, null, EvaluationReason::error(EvaluationReason::MALFORMED_FLAG_ERROR)); + self::assertEquals($detail, $result->getDetail()); self::assertEquals(array(), $result->getPrerequisiteEvents()); } @@ -517,7 +642,7 @@ public function testSegmentMatchClauseRetrievesSegmentFromStore() $result = $feature->evaluate($user, $requester); - self::assertTrue($result->getValue()); + self::assertTrue($result->getDetail()->getValue()); } public function testSegmentMatchClauseFallsThroughWithNoErrorsIfSegmentNotFound() @@ -531,7 +656,7 @@ public function testSegmentMatchClauseFallsThroughWithNoErrorsIfSegmentNotFound( $result = $feature->evaluate($user, $requester); - self::assertFalse($result->getValue()); + self::assertFalse($result->getDetail()->getValue()); } private function booleanFlagWithClauses($clauses) diff --git a/tests/FeatureFlagsStateTest.php b/tests/FeatureFlagsStateTest.php index 9b97d0a4b..e584f1e13 100644 --- a/tests/FeatureFlagsStateTest.php +++ b/tests/FeatureFlagsStateTest.php @@ -2,7 +2,8 @@ namespace LaunchDarkly\Tests; use InvalidArgumentException; -use LaunchDarkly\EvalResult; +use LaunchDarkly\EvaluationDetail; +use LaunchDarkly\EvaluationReason; use LaunchDarkly\FeatureFlag; use LaunchDarkly\FeatureFlagsState; @@ -42,7 +43,7 @@ public function testCanGetFlagValue() { $flag = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); $state = new FeatureFlagsState(true); - $state->addFlag($flag, new EvalResult(0, 'value1', array())); + $state->addFlag($flag, new EvaluationDetail('value1', 0)); $this->assertEquals('value1', $state->getFlagValue('key1')); } @@ -54,13 +55,38 @@ public function testUnknownFlagReturnsNullValue() $this->assertNull($state->getFlagValue('key1')); } + public function testCanGetFlagReason() + { + $flag = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag, new EvaluationDetail('value1', 0, EvaluationReason::off()), true); + + $this->assertEquals(EvaluationReason::off(), $state->getFlagReason('key1')); + } + + public function testUnknownFlagReturnsNullReason() + { + $state = new FeatureFlagsState(true); + + $this->assertNull($state->getFlagReason('key1')); + } + + public function testReasonIsNullIfReasonsWereNotRecorded() + { + $flag = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); + $state = new FeatureFlagsState(true); + $state->addFlag($flag, new EvaluationDetail('value1', 0, EvaluationReason::off()), false); + + $this->assertNull($state->getFlagReason('key1')); + } + public function testCanConvertToValuesMap() { $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); $state = new FeatureFlagsState(true); - $state->addFlag($flag1, new EvalResult(0, 'value1', array())); - $state->addFlag($flag2, new EvalResult(0, 'value2', array())); + $state->addFlag($flag1, new EvaluationDetail('value1', 0)); + $state->addFlag($flag2, new EvaluationDetail('value2', 0)); $expected = array('key1' => 'value1', 'key2' => 'value2'); $this->assertEquals($expected, $state->toValuesMap()); @@ -71,8 +97,8 @@ public function testCanConvertToJson() $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); $state = new FeatureFlagsState(true); - $state->addFlag($flag1, new EvalResult(0, 'value1', array())); - $state->addFlag($flag2, new EvalResult(1, 'value2', array())); + $state->addFlag($flag1, new EvaluationDetail('value1', 0)); + $state->addFlag($flag2, new EvaluationDetail('value2', 1)); $expected = array( 'key1' => 'value1', @@ -100,8 +126,8 @@ public function testJsonEncodeUsesCustomSerializer() $flag1 = FeatureFlag::decode(FeatureFlagsStateTest::$flag1Json); $flag2 = FeatureFlag::decode(FeatureFlagsStateTest::$flag2Json); $state = new FeatureFlagsState(true); - $state->addFlag($flag1, new EvalResult(0, 'value1', array())); - $state->addFlag($flag2, new EvalResult(1, 'value2', array())); + $state->addFlag($flag1, new EvaluationDetail('value1', 0)); + $state->addFlag($flag2, new EvaluationDetail('value2', 1)); $expected = $state->jsonSerialize(); $json = json_encode($state); diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php index e523bd3e2..73aa68d74 100644 --- a/tests/LDClientTest.php +++ b/tests/LDClientTest.php @@ -2,6 +2,7 @@ namespace LaunchDarkly\Tests; use InvalidArgumentException; +use LaunchDarkly\EvaluationReason; use LaunchDarkly\FeatureFlag; use LaunchDarkly\FeatureRequester; use LaunchDarkly\LDClient; @@ -16,10 +17,10 @@ public function testDefaultCtor() $this->assertInstanceOf(LDClient::class, new LDClient("BOGUS_SDK_KEY")); } - public function testVariationReturnsFlagValue() + private function makeOffFlagWithValue($key, $value) { $flagJson = array( - 'key' => 'feature', + 'key' => $key, 'version' => 100, 'deleted' => false, 'on' => false, @@ -28,21 +29,39 @@ public function testVariationReturnsFlagValue() 'rules' => array(), 'offVariation' => 1, 'fallthrough' => array('variation' => 0), - 'variations' => array('fall', 'off', 'on'), + 'variations' => array('FALLTHROUGH', $value), 'salt' => '' ); - $flag = FeatureFlag::decode($flagJson); + return FeatureFlag::decode($flagJson); + } + public function testVariationReturnsFlagValue() + { + $flag = $this->makeOffFlagWithValue('feature', 'value'); MockFeatureRequester::$flags = array('feature' => $flag); $client = new LDClient("someKey", array( 'feature_requester_class' => MockFeatureRequester::class, 'events' => false )); - $builder = new LDUserBuilder(3); - $user = $builder->build(); - $value = $client->variation('feature', $user, 'default'); - $this->assertEquals('off', $value); + $value = $client->variation('feature', new LDUser('userkey'), 'default'); + $this->assertEquals('value', $value); + } + + public function testVariationDetailReturnsFlagValue() + { + $flag = $this->makeOffFlagWithValue('feature', 'value'); + MockFeatureRequester::$flags = array('feature' => $flag); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $detail = $client->variationDetail('feature', new LDUser('userkey'), 'default'); + $this->assertEquals('value', $detail->getValue()); + $this->assertFalse($detail->isDefaultValue()); + $this->assertEquals(1, $detail->getVariationIndex()); + $this->assertEquals(EvaluationReason::off(), $detail->getReason()); } public function testVariationReturnsDefaultForUnknownFlag() @@ -53,9 +72,22 @@ public function testVariationReturnsDefaultForUnknownFlag() 'events' => false )); - $builder = new LDUserBuilder(3); - $user = $builder->build(); - $this->assertEquals('argdef', $client->variation('foo', $user, 'argdef')); + $this->assertEquals('argdef', $client->variation('foo', new LDUser('userkey'), 'argdef')); + } + + public function testVariationDetailReturnsDefaultForUnknownFlag() + { + MockFeatureRequester::$flags = array(); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $detail = $client->variationDetail('foo', new LDUser('userkey'), 'default'); + $this->assertEquals('default', $detail->getValue()); + $this->assertTrue($detail->isDefaultValue()); + $this->assertNull($detail->getVariationIndex()); + $this->assertEquals(EvaluationReason::error(EvaluationReason::FLAG_NOT_FOUND_ERROR), $detail->getReason()); } public function testVariationReturnsDefaultFromConfigurationForUnknownFlag() @@ -67,12 +99,60 @@ public function testVariationReturnsDefaultFromConfigurationForUnknownFlag() 'defaults' => array('foo' => 'fromarray') )); - $builder = new LDUserBuilder(3); - $user = $builder->build(); - $this->assertEquals('fromarray', $client->variation('foo', $user, 'argdef')); + $this->assertEquals('fromarray', $client->variation('foo', new LDUser('userkey'), 'argdef')); } public function testVariationSendsEvent() + { + $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->variation('flagkey', new LDUser('userkey'), '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('flagkey', $event['key']); + $this->assertEquals($flag->getVersion(), $event['version']); + $this->assertEquals('flagvalue', $event['value']); + $this->assertEquals(1, $event['variation']); + $this->assertEquals($user, $event['user']); + $this->assertEquals('default', $event['default']); + $this->assertFalse(isset($event['reason'])); + } + + public function testVariationDetailSendsEvent() + { + $flag = $this->makeOffFlagWithValue('FUCKINGWEIRDflagkey', 'flagvalue'); + MockFeatureRequester::$flags = array('FUCKINGWEIRDflagkey' => $flag); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => true + )); + + $user = new LDUser('userkey'); + $client->variationDetail('FUCKINGWEIRDflagkey', $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($flag->getVersion(), $event['version']); + $this->assertEquals('flagvalue', $event['value']); + $this->assertEquals(1, $event['variation']); + $this->assertEquals($user, $event['user']); + $this->assertEquals('default', $event['default']); + $this->assertEquals(array('kind' => 'OFF'), $event['reason']); + } + + public function testVariationSendsEventForUnknownFlag() { MockFeatureRequester::$flags = array(); $client = new LDClient("someKey", array( @@ -80,12 +160,44 @@ public function testVariationSendsEvent() 'events' => true )); - $builder = new LDUserBuilder(3); - $user = $builder->build(); - $client->variation('foo', $user, 'argdef'); + $user = new LDUser('userkey'); + $client->variation('flagkey', new LDUser('userkey'), '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('flagkey', $event['key']); + $this->assertNull($event['version']); + $this->assertEquals('default', $event['value']); + $this->assertNull($event['variation']); + $this->assertEquals($user, $event['user']); + $this->assertEquals('default', $event['default']); + $this->assertFalse(isset($event['reason'])); + } + + public function testVariationDetailSendsEventForUnknownFlag() + { + MockFeatureRequester::$flags = array(); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => true + )); + + $user = new LDUser('userkey'); + $client->variationDetail('flagkey', new LDUser('userkey'), '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('flagkey', $event['key']); + $this->assertNull($event['version']); + $this->assertEquals('default', $event['value']); + $this->assertNull($event['variation']); + $this->assertEquals($user, $event['user']); + $this->assertEquals('default', $event['default']); + $this->assertEquals(array('kind' => 'ERROR', 'errorKind' => 'FLAG_NOT_FOUND'), $event['reason']); } public function testAllFlagsReturnsFlagValues() @@ -164,6 +276,53 @@ public function testAllFlagsStateReturnsState() $this->assertEquals($expectedState, $state->jsonSerialize()); } + public function testAllFlagsStateReturnsStateWithReasons() + { + $flagJson = array( + 'key' => 'feature', + 'version' => 100, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => 1, + 'fallthrough' => array('variation' => 0), + 'variations' => array('fall', 'off', 'on'), + 'salt' => '', + 'trackEvents' => true, + 'debugEventsUntilDate' => 1000 + ); + $flag = FeatureFlag::decode($flagJson); + + MockFeatureRequester::$flags = array('feature' => $flag); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $builder = new LDUserBuilder(3); + $user = $builder->build(); + $state = $client->allFlagsState($user, array('withReasons' => true)); + + $this->assertTrue($state->isValid()); + $this->assertEquals(array('feature' => 'off'), $state->toValuesMap()); + $expectedState = array( + 'feature' => 'off', + '$flagsState' => array( + 'feature' => array( + 'variation' => 1, + 'version' => 100, + 'trackEvents' => true, + 'debugEventsUntilDate' => 1000, + 'reason' => array('kind' => 'OFF') + ) + ), + '$valid' => true + ); + $this->assertEquals($expectedState, $state->jsonSerialize()); + } + public function testAllFlagsStateCanFilterForClientSideFlags() { $flagJson = array('key' => 'server-side-1', 'version' => 1, 'on' => false, 'salt' => '', 'deleted' => false, From 5d8e2b065066b9c6166f9a9f7324238170a25d78 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 16:53:32 -0700 Subject: [PATCH 11/17] add another evaluation test --- tests/LDClientTest.php | 47 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php index 73aa68d74..dffc3afe2 100644 --- a/tests/LDClientTest.php +++ b/tests/LDClientTest.php @@ -35,6 +35,24 @@ private function makeOffFlagWithValue($key, $value) return FeatureFlag::decode($flagJson); } + private function makeFlagThatEvaluatesToNull($key) + { + $flagJson = array( + 'key' => $key, + 'version' => 100, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => null, + 'fallthrough' => array('variation' => 0), + 'variations' => array('FALLTHROUGH', $value), + 'salt' => '' + ); + return FeatureFlag::decode($flagJson); + } + public function testVariationReturnsFlagValue() { $flag = $this->makeOffFlagWithValue('feature', 'value'); @@ -64,6 +82,35 @@ public function testVariationDetailReturnsFlagValue() $this->assertEquals(EvaluationReason::off(), $detail->getReason()); } + public function testVariationReturnsDefaultIfFlagEvaluatesToNull() + { + $flag = $this->makeFlagThatEvaluatesToNull('feature'); + MockFeatureRequester::$flags = array('feature' => $flag); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $value = $client->variation('feature', new LDUser('userkey'), 'default'); + $this->assertEquals('default', $value); + } + + public function testVariationDetailReturnsDefaultIfFlagEvaluatesToNull() + { + $flag = $this->makeFlagThatEvaluatesToNull('feature'); + MockFeatureRequester::$flags = array('feature' => $flag); + $client = new LDClient("someKey", array( + 'feature_requester_class' => MockFeatureRequester::class, + 'events' => false + )); + + $detail = $client->variationDetail('feature', new LDUser('userkey'), 'default'); + $this->assertEquals('default', $detail->getValue()); + $this->assertTrue($detail->isDefaultValue()); + $this->assertNull($detail->getVariationIndex()); + $this->assertEquals(EvaluationReason::off(), $detail->getReason()); + } + public function testVariationReturnsDefaultForUnknownFlag() { MockFeatureRequester::$flags = array(); From 19770970ca0e79c810b21ba313f6f221f3ab9f46 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 16:58:27 -0700 Subject: [PATCH 12/17] linter --- src/LaunchDarkly/EvaluationDetail.php | 8 ++++---- src/LaunchDarkly/LDClient.php | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/LaunchDarkly/EvaluationDetail.php b/src/LaunchDarkly/EvaluationDetail.php index 7983ba1a1..b2ffb970b 100644 --- a/src/LaunchDarkly/EvaluationDetail.php +++ b/src/LaunchDarkly/EvaluationDetail.php @@ -27,7 +27,7 @@ public function __construct($value, $variationIndex, $reason = null) /** * Returns the value of the flag variation for the user. - * + * * @return mixed */ public function getValue() @@ -38,7 +38,7 @@ public function getValue() /** * Returns the index of the flag variation for the user, e.g. 0 for the first variation - * or null if it was the default value. - * + * * @return int | null */ public function getVariationIndex() @@ -48,7 +48,7 @@ public function getVariationIndex() /** * Returns information about how the flag value was calculated. - * + * * @return EvaluationReason */ public function getReason() @@ -58,7 +58,7 @@ public function getReason() /** * Returns true if the flag evaluated to the default value, rather than one of its variations. - * + * * @return bool */ public function isDefaultValue() diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index 7ff437e21..0c19e17bf 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -142,7 +142,7 @@ public function variation($key, $user, $default = false) * Calculates the value of a feature flag, and returns an object that describes the way the * value was determined. The "reason" property in the result will also be included in * analytics events, if you are capturing detailed event data for this flag. - * + * * @param string $key The unique key for the feature flag * @param LDUser $user The end user requesting the flag * @param mixed $default The default value of the flag @@ -161,13 +161,14 @@ public function variationDetail($key, $user, $default = false) * @param mixed $default * @param bool $includeReasonsInEvents */ - private function variationDetailInternal($key, $user, $default, $includeReasonsInEvents) { + private function variationDetailInternal($key, $user, $default, $includeReasonsInEvents) + { $default = $this->_get_default($key, $default); - $errorResult = function($errorKind) use ($key, $default) { + $errorResult = function ($errorKind) use ($key, $default) { return new EvaluationDetail($default, null, EvaluationReason::error($errorKind)); }; - $sendEvent = function($detail, $flag) use ($key, $user, $default, $includeReasonsInEvents) { + $sendEvent = function ($detail, $flag) use ($key, $user, $default, $includeReasonsInEvents) { if ($this->isOffline() || !$this->_send_events) { return; } From 54759db95c42eafb92e99b63d46e4ebfbaaea79c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 24 Aug 2018 17:03:13 -0700 Subject: [PATCH 13/17] fix test method --- tests/LDClientTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/LDClientTest.php b/tests/LDClientTest.php index dffc3afe2..6893a3bd4 100644 --- a/tests/LDClientTest.php +++ b/tests/LDClientTest.php @@ -47,7 +47,7 @@ private function makeFlagThatEvaluatesToNull($key) 'rules' => array(), 'offVariation' => null, 'fallthrough' => array('variation' => 0), - 'variations' => array('FALLTHROUGH', $value), + 'variations' => array('none'), 'salt' => '' ); return FeatureFlag::decode($flagJson); From 39d5105a262553202349a33e7b77ad598065e929 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 29 Aug 2018 11:46:19 -0700 Subject: [PATCH 14/17] fix for ch22995 - include prereq value in event even if prereq is off --- src/LaunchDarkly/FeatureFlag.php | 24 ++++++--------- tests/FeatureFlagTest.php | 53 ++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 14 deletions(-) diff --git a/src/LaunchDarkly/FeatureFlag.php b/src/LaunchDarkly/FeatureFlag.php index 1ea5af29a..46123f8a4 100644 --- a/src/LaunchDarkly/FeatureFlag.php +++ b/src/LaunchDarkly/FeatureFlag.php @@ -109,12 +109,9 @@ public function isOn() */ public function evaluate($user, $featureRequester, $includeReasonsInEvents = false) { - if ($this->isOn()) { - $prereqEvents = array(); - $detail = $this->evaluateInternal($user, $featureRequester, $prereqEvents, $includeReasonsInEvents); - return new EvalResult($detail, $prereqEvents); - } - return new EvalResult($this->getOffValue(EvaluationReason::off()), array()); + $prereqEvents = array(); + $detail = $this->evaluateInternal($user, $featureRequester, $prereqEvents, $includeReasonsInEvents); + return new EvalResult($detail, $prereqEvents); } /** @@ -126,6 +123,10 @@ public function evaluate($user, $featureRequester, $includeReasonsInEvents = fal */ private function evaluateInternal($user, $featureRequester, &$events, $includeReasonsInEvents) { + if (!$this->isOn()) { + return $this->getOffValue(EvaluationReason::off()); + } + $prereqFailureReason = $this->checkPrerequisites($user, $featureRequester, $events, $includeReasonsInEvents); if ($prereqFailureReason !== null) { return $this->getOffValue($prereqFailureReason); @@ -170,19 +171,14 @@ private function checkPrerequisites($user, $featureRequester, &$events, $include $prereqFeatureFlag = $featureRequester->getFeature($prereq->getKey()); if ($prereqFeatureFlag == null) { $prereqOk = false; - } elseif ($prereqFeatureFlag->isOn()) { + } else { $prereqEvalResult = $prereqFeatureFlag->evaluateInternal($user, $featureRequester, $events, $includeReasonsInEvents); $variation = $prereq->getVariation(); - if ($prereqEvalResult->getVariationIndex() !== $variation) { + if (!$prereqFeatureFlag->isOn() || $prereqEvalResult->getVariationIndex() !== $variation) { $prereqOk = false; } - } else { - $prereqOk = false; - } - if ($prereqFeatureFlag) { array_push($events, Util::newFeatureRequestEvent($prereq->getKey(), $user, - $prereqEvalResult ? $prereqEvalResult->getVariationIndex() : null, - $prereqEvalResult ? $prereqEvalResult->getValue() : null, + $prereqEvalResult->getVariationIndex(), $prereqEvalResult->getValue(), null, $prereqFeatureFlag->getVersion(), $this->_key, ($includeReasonsInEvents && $prereqEvalResult) ? $prereqEvalResult->getReason() : null )); diff --git a/tests/FeatureFlagTest.php b/tests/FeatureFlagTest.php index 2ea5f9e03..f0f09f02e 100644 --- a/tests/FeatureFlagTest.php +++ b/tests/FeatureFlagTest.php @@ -314,6 +314,59 @@ public function testFlagReturnsOffVariationIfPrerequisiteIsNotFound() self::assertEquals(array(), $result->getPrerequisiteEvents()); } + public function testFlagReturnsOffVariationAndEventIfPrerequisiteIsOff() + { + $flag0Json = array( + 'key' => 'feature0', + 'version' => 1, + 'deleted' => false, + 'on' => true, + 'targets' => array(), + 'prerequisites' => array( + array('key' => 'feature1', 'variation' => 1) + ), + 'rules' => array(), + 'offVariation' => 1, + 'fallthrough' => array('variation' => 0), + 'variations' => array('fall', 'off', 'on'), + 'salt' => '' + ); + $flag1Json = array( + 'key' => 'feature1', + 'version' => 2, + 'deleted' => false, + 'on' => false, + 'targets' => array(), + 'prerequisites' => array(), + 'rules' => array(), + 'offVariation' => 1, + // note that even though it returns the desired variation, it is still off and therefore not a match + 'fallthrough' => array('variation' => 0), + 'variations' => array('nogo', 'go'), + 'salt' => '' + ); + $flag0 = FeatureFlag::decode($flag0Json); + $flag1 = FeatureFlag::decode($flag1Json); + $ub = new LDUserBuilder('x'); + $user = $ub->build(); + $requester = new MockFeatureRequesterForFeature(); + $requester->key = $flag1->getKey(); + $requester->val = $flag1; + + $result = $flag0->evaluate($user, $requester); + $detail = new EvaluationDetail('off', 1, EvaluationReason::prerequisiteFailed('feature1')); + self::assertEquals($detail, $result->getDetail()); + + $events = $result->getPrerequisiteEvents(); + self::assertEquals(1, count($events)); + $event = $events[0]; + self::assertEquals('feature', $event['kind']); + self::assertEquals($flag1->getKey(), $event['key']); + self::assertEquals('go', $event['value']); + self::assertEquals($flag1->getVersion(), $event['version']); + self::assertEquals($flag0->getKey(), $event['prereqOf']); + } + public function testFlagReturnsOffVariationAndEventIfPrerequisiteIsNotMet() { $flag0Json = array( From 180679f05ae8c112d6e0b479beaa9539131d5d6d Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 24 Sep 2018 16:41:12 -0700 Subject: [PATCH 15/17] cache flag data in allFlags --- src/LaunchDarkly/LDClient.php | 5 +- .../PreloadedFeatureRequester.php | 59 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 src/LaunchDarkly/PreloadedFeatureRequester.php diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index 2df377edd..2c5743df4 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -350,6 +350,9 @@ public function allFlagsState($user, $options = array()) return new FeatureFlagsState(false); } + $preloadedRequester = new PreloadedFeatureRequester($this->_featureRequester, $flags); + // This saves us from doing repeated queries for prerequisite flags during evaluation + $state = new FeatureFlagsState(true); $clientOnly = isset($options['clientSideOnly']) && $options['clientSideOnly']; $withReasons = isset($options['withReasons']) && $options['withReasons']; @@ -357,7 +360,7 @@ public function allFlagsState($user, $options = array()) if ($clientOnly && !$flag->isClientSide()) { continue; } - $result = $flag->evaluate($user, $this->_featureRequester); + $result = $flag->evaluate($user, $preloadedRequester); $state->addFlag($flag, $result->getDetail(), $withReasons); } return $state; diff --git a/src/LaunchDarkly/PreloadedFeatureRequester.php b/src/LaunchDarkly/PreloadedFeatureRequester.php new file mode 100644 index 000000000..9e739ef14 --- /dev/null +++ b/src/LaunchDarkly/PreloadedFeatureRequester.php @@ -0,0 +1,59 @@ +_baseRequester = $baseRequester; + $this->_knownFeatures = $knownFeatures; + } + + /** + * Gets feature data from cached values + * + * @param $key string feature key + * @return FeatureFlag|null The decoded FeatureFlag, or null if missing + */ + public function getFeature($key) + { + if (isset($this->_knownFeatures[$key])) { + return $this->_knownFeatures[$key]; + } + return null; + } + + /** + * Gets segment data from the regular feature requester + * + * @param $key string segment key + * @return Segment|null The decoded Segment, or null if missing + */ + public function getSegment($key) + { + return $this->_baseRequester->getSegment($key); + } + + /** + * Gets all features from cached values + * + * @return array()|null The decoded FeatureFlags, or null if missing + */ + public function getAllFeatures() + { + return $this->_knownFeatures; + } +} From 15720d0694a6bf19b9f5264c5070590b4b81f26c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 24 Sep 2018 16:51:05 -0700 Subject: [PATCH 16/17] rm unused imports --- src/LaunchDarkly/PreloadedFeatureRequester.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/LaunchDarkly/PreloadedFeatureRequester.php b/src/LaunchDarkly/PreloadedFeatureRequester.php index 9e739ef14..ba307f6c0 100644 --- a/src/LaunchDarkly/PreloadedFeatureRequester.php +++ b/src/LaunchDarkly/PreloadedFeatureRequester.php @@ -1,13 +1,6 @@ Date: Tue, 25 Sep 2018 11:41:24 -0700 Subject: [PATCH 17/17] version 3.4.1 --- CHANGELOG.md | 4 ++++ VERSION | 2 +- src/LaunchDarkly/LDClient.php | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3309e3b9a..7f9f4613e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to the LaunchDarkly PHP SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [3.4.1] - 2018-09-25 +### Fixed: +- Improved the performance of `allFlags`/`allFlagsState` by not making redundant individual requests for prerequisite flags, when a flag is being evaluated that has prerequisites. Instead it will reuse the same flag data that it already obtained from LaunchDarkly in the "get all the flags" request. + ## [3.4.0] - 2018-09-04 ### Added: - The new `LDClient` method `variationDetail` allows you to evaluate a feature flag (using the same parameters as you would for `variation`) and receive more information about how the value was calculated. This information is returned in an object that contains both the result value and a "reason" object which will tell you, for instance, if the user was individually targeted for the flag or was matched by one of the flag's rules, or if the flag returned the default value due to an error. diff --git a/VERSION b/VERSION index 18091983f..47b322c97 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.4.0 +3.4.1 diff --git a/src/LaunchDarkly/LDClient.php b/src/LaunchDarkly/LDClient.php index ef3d57c27..2e23c55b0 100644 --- a/src/LaunchDarkly/LDClient.php +++ b/src/LaunchDarkly/LDClient.php @@ -12,7 +12,7 @@ class LDClient { const DEFAULT_BASE_URI = 'https://app.launchdarkly.com'; const DEFAULT_EVENTS_URI = 'https://events.launchdarkly.com'; - const VERSION = '3.4.0'; + const VERSION = '3.4.1'; /** @var string */ protected $_sdkKey;