diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml new file mode 100644 index 0000000..991bfff --- /dev/null +++ b/.github/workflows/php.yml @@ -0,0 +1,30 @@ +name: PHP Composer + +on: + push: + branches: ["master"] + pull_request: + branches: ["master"] + +permissions: + contents: read + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - name: "Checkout" + uses: actions/checkout@v3 + + - name: "Install PHP" + uses: shivammathur/setup-php@v2 + with: + php-version: "8.3" + tools: composer + + - name: Install dependencies + run: composer install --no-interaction + + - name: Run test suite + run: composer test diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..d1502b0 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +vendor/ +composer.lock diff --git a/BRANCHES.md b/BRANCHES.md deleted file mode 100644 index 8556ec8..0000000 --- a/BRANCHES.md +++ /dev/null @@ -1,9 +0,0 @@ -# Branches - -There are three branches in this repo. - -- etsy -- A snapshot of the code from the actual Etsy sourcecode. - -- master -- The same as `etsy` except with most (all?) of the dependencies on other Etsy code stripped out. - -- generalized -- A quick, untested attempt to generalize the code for non-Etsy contexts. diff --git a/Feature.php b/Feature.php deleted file mode 100644 index 5f66720..0000000 --- a/Feature.php +++ /dev/null @@ -1,250 +0,0 @@ -isEnabled(); - } - - /** - * Test whether the named feature is enabled for a given - * user. This method should only be used when we want to bucket - * based on a user other than the current logged in user, e.g. if - * we are bucketing different listings based on their owner. - * - * @static - * @param string $name the config key for this feature. - * - * @param $user A user object whose id will be combined with $name - * and hashed to get the bucketing. - * - * @return bool - */ - public static function isEnabledFor($name, $user) { - return self::fromConfig($name)->isEnabledFor($user); - } - - /** - * Test whether the named feature is enabled for a given - * arbitrary string. This method should only be used when we want to bucket - * based on something other than a user, e.g. shops, teams, treasuries, tags, etc. - * - * @static - * @param string $name the config key for this feature. - * - * @param $string A string which will be combined with $name - * and hashed to get the bucketing. - * - * @return bool - */ - public static function isEnabledBucketingBy($name, $string) { - return self::fromConfig($name)->isEnabledBucketingBy($string); - } - - /** - * Get the name of the A/B variant for the named feature for the - * current user. Logs an error if called when isEnabled($name) - * doesn't return true. (I.e. calls to this method should only - * occur in blocks guarded by an isEnabled check.) - * - * Also logs an error if 'enabled' is 'on' for the named feature - * since there should be no variant-dependent code left when a - * feature has been fully enabled. To clean up a finished - * experiment, first set 'enabled' to the name of the winning - * variant. - * - * @static - * @param string $name the config key for the feature. - */ - public static function variant($name) { - return self::fromConfig($name)->variant(); - } - - /** - * Get the name of the A/B variant for the named feature for the - * given user. This method should only be used when we want to - * bucket based on a user other than the current logged in user, - * e.g. if we are bucketing different listings based on their - * owner. - * - * Logs an error if called when isEnabledFor($name, $user) doesn't - * return true. (I.e. calls to this method should only occur in - * blocks guarded by an isEnabledFor check.) - - * Also logs an error if 'enabled' is 'on' for the named feature - * since there should be no variant-dependent code left when a - * feature has been fully enabled. To clean up a finished - * experiment, first set 'enabled' to the name of the winning - * variant. - * - * @static - * - * @param string $name the config key for the feature. - * - * @param $user A user object whose id will be combined with $name - * and hashed to get the bucketing. - */ - public static function variantFor($name, $user) { - return self::fromConfig($name)->variantFor($user); - } - - /** - * Get the name of the A/B variant for the named feature, - * bucketing by the given bucketing ID. (For other checks such as - * admin, and user whitelists uses the current user which may or - * may not make sense. If it doesn't make sense, don't configure - * the feature to use those mechanisms.) Logs an error if called - * when isEnabled($name) doesn't return true. (I.e. calls to this - * method should only occur in blocks guarded by an isEnabled - * check.) - * - * Also logs an error if 'enabled' is 'on' for the named feature - * since there should be no variant-dependent code left when a - * feature has been fully enabled. To clean up a finished - * experiment, first set 'enabled' to the name of the winning - * variant. - * - * @static - * - * @param string $name the config key for the feature. - * - * @param string $bucketingID A string to use as the bucketing ID. - */ - public static function variantBucketingBy($name, $bucketingID) { - return self::fromConfig($name)->variantBucketingBy($bucketingID); - } - - /* - * Description of the feature. - */ - public static function description ($name) { - return self::fromConfig($name)->description(); - } - - /** - * Get data related to a Feature name: config must be nested - * under the Feature name, in an array key named 'data'. - * @param string $name the Feature key to find data for - * @param mixed $default what to return if not defined - * - * @return mixed - */ - public static function data($name, $default = array()) { - return self::world()->configValue("$name.data", $default); - } - - /** - * Get data linked to a Feature name, specific for the enabled variant. - * Nest data in an array named 'data' with a key for each variant. - * @param string $name the Feature key to find data for - * @param mixed $default what to return if not found - * - * @return mixed - */ - public static function variantData($name, $default = array()) { - $data = self::data($name); - $variant = self::variant($name); - return isset($data[$variant]) ? $data[$variant] : $default; - } - - /** - * Get the named feature object. We cache the object after - * building it from the config stanza to speed lookups. - * - * @static - * - * @param $name name of the feature. Used as a key into the global config array - * - * @return Feature_Config - */ - private static function fromConfig($name) { - if (array_key_exists($name, self::$configCache)) { - return self::$configCache[$name]; - } else { - $world = self::world(); - $stanza = $world->configValue($name); - return self::$configCache[$name] = new Feature_Config($name, $stanza, $world); - } - } - - /** - * N.B. This method is for testing only. (The issue is that once a - * Feature has been checked once, the result of the check is - * cached but in tests we need to change the configuration and - * have those changes be reflected in feature checks.) - */ - public static function clearCacheForTests() { - self::$configCache = array(); - } - - - /** - * Get the list of selections that have been made as an array of - * (feature_name, variant_name, selector) arrays. This can be used - * to record information about what features were associated with - * what variants and why during the course of handling a request. - */ - public static function selections () { - return self::world()->selections(); - } - - /** - * This API always uses the default World. Feature_Config takes - * the world as an argument in order to ease unit testing. - */ - private static function world () { - if (!isset(self::$defaultWorld)) { - self::$defaultWorld = new Feature_World(new Feature_Logger()); - } - return self::$defaultWorld; - } -} diff --git a/Feature/Config.php b/Feature/Config.php deleted file mode 100644 index 98c0d46..0000000 --- a/Feature/Config.php +++ /dev/null @@ -1,512 +0,0 @@ -_name = $name; - $this->_cache = array(); - $this->_world = $world; - - // Special case to save some memory--if the value is just a - // string that is the same as setting enabled to that variant - // (typically 'on' or 'off' but possibly another variant - // name). This reduces the number of array objects we have to - // create when reading the config file. - if (is_null($stanza)) { - $stanza = array(self::ENABLED => self::OFF); - } elseif (is_string($stanza)) { - $stanza = array(self::ENABLED => $stanza); - } - - // Pull stuff from the config stanza. - $this->_description = $this->parseDescription($stanza); - $this->_enabled = $this->parseEnabled($stanza); - $this->_users = $this->parseUsersOrGroups($stanza, self::USERS); - $this->_groups = $this->parseUsersOrGroups($stanza, self::GROUPS); - $this->_adminVariant = $this->parseVariantName($stanza, self::ADMIN); - $this->_internalVariant = $this->parseVariantName($stanza, self::INTERNAL); - $this->_public_url_override = $this->parsePublicURLOverride($stanza); - $this->_bucketing = $this->parseBucketBy($stanza); - - // Put the _enabled value into a more useful form for actually doing bucketing. - $this->_percentages = $this->computePercentages(); - } - - //////////////////////////////////////////////////////////////////////// - // Public API, though note that Feature.php is the only code that - // should be using this class directly. - - /* - * Is this feature enabled for the default id and the logged in - * user, if any? - */ - public function isEnabled () { - $bucketingID = $this->bucketingID(); - $userID = $this->_world->userID(); - return $this->chooseVariant($bucketingID, $userID, false) !== self::OFF; - } - - /* - * What variant is enabled for the default id and the logged in - * user, if any? - */ - public function variant () { - $bucketingID = $this->bucketingID(); - $userID = $this->_world->userID(); - return $this->chooseVariant($bucketingID, $userID, true);; - } - - /* - * Is this feature enabled for the given user? - */ - public function isEnabledFor ($user) { - $userID = $this->getUserIdFrom($user); - return $this->chooseVariant($userID, $userID, false) !== self::OFF; - } - - /* - * Is this feature enabled, bucketing on the given bucketing - * ID? (Other methods of enabling a feature and specifying a - * variant such as users, groups, and query parameters, will still - * work.) - */ - public function isEnabledBucketingBy ($bucketingID) { - $userID = $this->_world->userID(); - return $this->chooseVariant($bucketingID, $userID, false) !== self::OFF; - } - - /* - * What variant is enabled for the given user? - */ - public function variantFor ($user) { - $userID = $this->getUserIdFrom($user); - return $this->chooseVariant($userID, $userID, true); - } - - /* - * What variant is enabled, bucketing on the given bucketing ID, - * if any? - */ - public function variantBucketingBy ($bucketingID) { - $userID = $this->_world->userID(); - return $this->chooseVariant($bucketingID, $userID, true);; - } - - /* - * Description of the feature. - */ - public function description () { - return $this->_description; - } - - - //////////////////////////////////////////////////////////////////////// - // Internals - - /* - * Accept different user objects and return user_id - */ - private function getUserIdFrom($user) { - if ($user instanceof REST_User) { - // $user->user_id is protected so not accessible - return $user->getUserId(); - } - return $user->user_id; - } - - /* - * Get the name of the variant we should use. Returns OFF if the - * feature is not enabled for $id. When $inVariantMethod is - * true will also check the conditions that should hold for a - * correct call to variant or variantFor: they should not be - * called for features that are completely enabled (i.e. 'enabled' - * => 'on') since all such variant-specific code should have been - * cleaned up before changing the config and they should not be - * called if the feature is, in fact, disabled for the given id - * since those two methods should always be guarded by an - * isEnabled/isEnabledFor call. - * - * @param $bucketingID the id used to assign a variant based on - * the percentage of users that should see different variants. - * - * @param $userID the identity of the user to be used for the - * special 'admin', 'users', and 'groups' access checks. - * - * @param $inVariantMethod were we called from variant or - * variantFor, in which case we want to perform some certain - * sanity checks to make sure the code is being used correctly. - */ - private function chooseVariant ($bucketingID, $userID, $inVariantMethod) { - if ($inVariantMethod && $this->_enabled === self::ON) { - $this->error("Variant check when fully enabled"); - } - - if (is_string($this->_enabled)) { - // When enabled is on, off, or a variant name, that's the - // end of the story. - return $this->_enabled; - } else { - if (is_null($bucketingID)) { - throw new InvalidArgumentException( - "no bucketing ID supplied. if testing, configure feature " . - "with enabled => 'on' or 'off', feature name = " . - $this->_name - ); - } - - $bucketingID = (string)$bucketingID; - if (array_key_exists($bucketingID, $this->_cache)) { - // Note that this caching is not just an optimization: - // it prevents us from double logging a single - // feature--we only want to log each distinct checked - // feature once. - // - // The caching also affects the semantics when we use - // random bucketing (rather than hashing the id), i.e. - // 'random' => 'true', by making the variant and - // enabled status stable within a request. - return $this->_cache[$bucketingID]; - } else { - list($v, $selector) = - $this->variantFromURL($userID) ?: - $this->variantForUser($userID) ?: - $this->variantForGroup($userID) ?: - $this->variantForAdmin($userID) ?: - $this->variantForInternal() ?: - $this->variantByPercentage($bucketingID) ?: - array(self::OFF, 'w'); - - if ($inVariantMethod && $v === self::OFF) { - $this->error("Variant check outside enabled check"); - } - - $this->_world->log($this->_name, $v, $selector); - - return $this->_cache[$bucketingID] = $v; - } - } - } - - /* - * Return the globally accessible ID used by the one-arg isEnabled - * and variant methods based on the feature's bucketing property. - */ - private function bucketingID () { - switch ($this->_bucketing) { - case self::UAID: - case self::RANDOM: - // In the RANDOM case we still need a bucketing id to keep - // the assignment stable within a request. - // Note that when being run from outside of a web request (e.g. crons), - // there is no UAID, so we default to a static string - $uaid = $this->_world->uaid(); - return $uaid ? $uaid : "no uaid"; - case self::USER: - $userID = $this->_world->userID(); - // Not clear if this is right. There's an argument to be - // made that if we're bucketing by userID and the user is - // not logged in we should treat the feature as disabled. - return !is_null($userID) ? $userID : $this->_world->uaid(); - default: - throw new InvalidArgumentException("Bad bucketing: $this->bucketing"); - } - } - - /* - * For internal requests or if the feature has public_url_override - * set to true, a specific variant can be specified in the - * 'features' query parameter. In all other cases return false, - * meaning nothing was specified. Note that foo:off will turn off - * the 'foo' feature. - */ - private function variantFromURL ($userID) { - if ($this->_public_url_override or - $this->_world->isInternalRequest() or - $this->_world->isAdmin($userID) - ) { - $urlFeatures = $this->_world->urlFeatures(); - if ($urlFeatures) { - foreach (explode(',', $urlFeatures) as $f) { - $parts = explode(':', $f); - if ($parts[0] === $this->_name) { - return array(isset($parts[1]) ? $parts[1] : self::ON, 'o'); - } - } - } - } - return false; - } - - /* - * Get the variant this user should see, if one was configured, - * false otherwise. - */ - private function variantForUser ($userID) { - if ($this->_users) { - $name = $this->_world->userName($userID); - if ($name && array_key_exists($name, $this->_users)) { - return array($this->_users[$name], 'u'); - } - } - return false; - } - - /* - * Get the variant this user should see based on their group - * memberships, if one was configured, false otherwise. N.B. If - * the user is in multiple groups that are configured to see - * different variants, they'll get the variant for one of their - * groups but there's no saying which one. If this is a problem in - * practice we could make the configuration more complex. Or you - * can just provide a specific variant via the 'users' property. - */ - private function variantForGroup ($userID) { - if ($userID) { - foreach ($this->_groups as $groupID => $variant) { - if ($this->_world->inGroup($userID, $groupID)) { - return array($variant, 'g'); - } - } - } - return false; - } - - /* - * What variant, if any, should we return if the current user is - * an admin. - */ - private function variantForAdmin ($userID) { - if ($userID && $this->_adminVariant) { - if ($this->_world->isAdmin($userID)) { - return array($this->_adminVariant, 'a'); - } - } - return false; - } - - /* - * What variant, if any, should we return for internal requests. - */ - private function variantForInternal () { - if ($this->_internalVariant) { - if ($this->_world->isInternalRequest()) { - return array($this->_internalVariant, 'i'); - } - } - return false; - } - - /* - * Finally, the normal case: use the percentage of users who - * should see each variant to map a randomish number to a - * particular variant. - */ - private function variantByPercentage ($id) { - $n = 100 * $this->randomish($id); - foreach ($this->_percentages as $v) { - // === 100 check may not be necessary but I'm not good - // enough numerical analyst to be sure. - if ($n < $v[0] || $v[0] === 100) { - return array($v[1], 'w'); - } - } - return false; - } - - /* - * A randomish number in [0, 1) based on the feature name and $id - * unless we are bucketing completely at random. - */ - private function randomish ($id) { - return $this->_bucketing === self::RANDOM - ? $this->_world->random() : $this->_world->hash($this->_name . '-' . $id); - } - - //////////////////////////////////////////////////////////////////////// - // Configuration parsing - - private function parseDescription ($stanza) { - return Feature_Util::arrayGet($stanza, self::DESCRIPTION, 'No description.'); - } - - /* - * Parse the 'enabled' property of the feature's config stanza. - */ - private function parseEnabled ($stanza) { - - $enabled = Feature_Util::arrayGet($stanza, self::ENABLED, 0); - - if (is_numeric($enabled)) { - if ($enabled < 0) { - $this->error("enabled ($enabled) < 0"); - $enabled = 0; - } elseif ($enabled > 100) { - $this->error("enabled ($enabled) > 100"); - $enabled = 100; - } - return array('on' => $enabled); - - } elseif (is_string($enabled) or is_array($enabled)) { - return $enabled; - } else { - $this->error("Malformed enabled property"); - } - } - - /* - * Returns an array of pairs with the first element of the pair - * being the upper-boundary of the variants percentage and the - * second element being the name of the variant. - */ - private function computePercentages () { - $total = 0; - $percentages = array(); - if (is_array($this->_enabled)) { - foreach ($this->_enabled as $variant => $percentage) { - if (!is_numeric($percentage) || $percentage < 0 || $percentage > 100) { - $this->error("Bad percentage $percentage"); - } - if ($percentage > 0) { - $total += $percentage; - $percentages[] = array($total, $variant); - } - if ($total > 100) { - $this->error("Total of percentages > 100: $total"); - } - } - } - return $percentages; - } - - /* - * Parse the value of the 'users' and 'groups' properties of the - * feature's config stanza, returning an array mappinng the user - * or group names to they variant they should see. - */ - private function parseUsersOrGroups ($stanza, $what) { - $value = Feature_Util::arrayGet($stanza, $what); - if (is_string($value) || is_numeric($value)) { - // Users are configrued with their user names. Groups as - // numeric ids. (Not sure if that's a great idea.) - return array($value => self::ON); - - } elseif (self::isList($value)) { - $result = array(); - foreach ($value as $who) { - $result[strtolower($who)] = self::ON; - } - return $result; - - } elseif (is_array($value)) { - $result = array(); - $bad_keys = is_array($this->_enabled) ? - array_keys(array_diff_key($value, $this->_enabled)) : - array(); - if (!$bad_keys) { - foreach ($value as $variant => $whos) { - foreach (self::asArray($whos) as $who) { - $result[strtolower($who)] = $variant; - } - } - return $result; - - } else { - $this->error("Unknown variants " . implode(', ', $bad_keys)); - } - } else { - return array(); - } - } - - /* - * Parse the variant name value for the 'admin' and 'internal' - * properties. If non-falsy, must be one of the keys in the - * enabled map unless enabled is 'on' or 'off'. - */ - private function parseVariantName ($stanza, $what) { - $value = Feature_Util::arrayGet($stanza, $what); - if ($value) { - if (is_array($this->_enabled)) { - if (array_key_exists($value, $this->_enabled)) { - return $value; - } else { - $this->error("Unknown variant $value"); - } - } else { - return $value; - } - } else { - return false; - } - } - - private function parsePublicURLOverride ($stanza) { - return Feature_Util::arrayGet($stanza, self::PUBLIC_URL_OVERRIDE, false); - } - - private function parseBucketBy ($stanza) { - return Feature_Util::arrayGet($stanza, self::BUCKETING, self::UAID); - } - - //////////////////////////////////////////////////////////////////////// - // Genericish utilities - - /* - * Is the given object an array value that could have been created - * with array(...) with no =>'s in the ...? - */ - private static function isList($a) { - return is_array($a) and array_keys($a) === range(0, count($a) - 1); - } - - private static function asArray ($x) { - return is_array($x) ? $x : array($x); - } - - private function error ($message) { - // IMPLEMENT FOR YOUR CONTEXT - } -} diff --git a/Feature/Instance.php b/Feature/Instance.php deleted file mode 100644 index 7de9458..0000000 --- a/Feature/Instance.php +++ /dev/null @@ -1,52 +0,0 @@ - (int)$value); - } else if (is_string($value)) { - $value = array('enabled' => $value); - } - - $enabled = Feature_Util::arrayGet($value, 'enabled', 0); - $users = self::expandUsersOrGroups(Feature_Util::arrayGet($value, 'users', array())); - $groups = self::expandUsersOrGroups(Feature_Util::arrayGet($value, 'groups', array())); - - if ($enabled === 'off') { - $spec['variants'][] = self::makeVariantWithUsersAndGroups('on', 0, $users, $groups); - $internal_url = false; - } else if (is_numeric($enabled)) { - $spec['variants'][] = self::makeVariantWithUsersAndGroups('on', (int)$enabled, $users, $groups); - } else if (is_string($enabled)) { - $spec['variants'][] = self::makeVariantWithUsersAndGroups($enabled, 100, $users, $groups); - $internal_url = false; - } else if (is_array($enabled)) { - foreach ($enabled as $v => $p) { - if (is_numeric($p)) { - // Kind of a kludge. $p had better be numeric and - // there have been configs deployed where it - // wasn't which breaks the Catapult config history - // scripts. This will just skip those. - $spec['variants'][] = self::makeVariantWithUsersAndGroups($v, $p, $users, $groups); - } - } - } - $spec['internal_url_override'] = $internal_url; - - if (array_key_exists('admin', $value)) { - $spec['admin'] = $value['admin']; - } - if (array_key_exists('internal', $value)) { - $spec['internal'] = $value['internal']; - } - if (array_key_exists('bucketing', $value)) { - $spec['bucketing'] = $value['bucketing']; - } - if (array_key_exists('internal', $value)) { - $spec['internal'] = $value['internal']; - } - if (array_key_exists('public_url_override', $value)) { - $spec['public_url_override'] = $value['public_url_override']; - } - - return $spec; - } - - private static function makeSpec ($key) { - return array( - 'key' => $key, - 'internal_url_override' => false, - 'public_url_override' => false, - 'bucketing' => 'uaid', - 'admin' => null, - 'internal' => null, - 'variants' => array()); - } - - private static function makeVariant ($name, $percentage) { - return array( - 'name' => $name, - 'percentage' => $percentage, - 'users' => array(), - 'groups' => array()); - } - - private static function makeVariantWithUsersAndGroups ($name, $percentage, $users, $groups) { - return array( - 'name' => $name, - 'percentage' => $percentage, - 'users' => self::extractForVariant($users, $name), - 'groups' => self::extractForVariant($groups, $name), - ); - } - - private static function extractForVariant ($usersOrGroups, $name) { - $result = array(); - foreach ($usersOrGroups as $thing => $variant) { - if ($variant == $name) { - $result[] = $thing; - } - } - return $result; - } - - // This is based on parseUsersOrGroups in Feature_Config. Probably - // this logic should be put in that class in a form that we can - // use. - private static function expandUsersOrGroups ($value) { - if (is_string($value) || is_numeric($value)) { - return array($value => Feature_Config::ON); - - } elseif (self::isList($value)) { - $result = array(); - foreach ($value as $who) { - $result[$who] = Feature_Config::ON; - } - return $result; - - } elseif (is_array($value)) { - $result = array(); - foreach ($value as $variant => $whos) { - foreach (self::asArray($whos) as $who) { - $result[$who] = $variant; - } - } - return $result; - - } else { - return array(); - } - } - - private static function isList($a) { - return is_array($a) and array_keys($a) === range(0, count($a) - 1); - } - - private static function asArray ($x) { - return is_array($x) ? $x : array($x); - } - -} \ No newline at end of file diff --git a/Feature/Lint.php b/Feature/Lint.php deleted file mode 100644 index 6d9a518..0000000 --- a/Feature/Lint.php +++ /dev/null @@ -1,224 +0,0 @@ - 100. - */ -class Feature_Lint { - - private $_checked; - private $_errors; - private $_path; - - public function __construct() { - $this->_checked = 0; - $this->_errors = array(); - $this->_path = array(); - $this->syntax_keys = array( - Feature_Config::ENABLED, - Feature_Config::USERS, - Feature_Config::GROUPS, - Feature_Config::ADMIN, - Feature_Config::INTERNAL, - Feature_Config::PUBLIC_URL_OVERRIDE, - Feature_Config::BUCKETING, - 'data', - ); - - $this->_legal_bucketing_values = array( - Feature_Config::UAID, - Feature_Config::USER, - Feature_Config::RANDOM, - ); - } - - public function run($file = null) { - $config = $this->fromFile($file); - $this->assert($config, "*** Bad configuration."); - $this->lintNested($config); - } - - public function checked() { - return $this->_checked; - } - - public function errors() { - return $this->_errors; - } - - private function fromFile($file) { - global $server_config; - $content = file_get_contents($file); - error_reporting(0); - $r = eval('?>' . $content); - error_reporting(-1); - if ($r === null) { - return $server_config; - } else if ($r === false) { - return false; - } else { - Logger::error("Wut? $r"); - return false; - } - } - - /* - * Recursively check nested feature configurations. Skips any keys - * that have a syntactic meaning which includes 'data'. - */ - private function lintNested($config) { - foreach ($config as $name => $stanza) { - if (!in_array($name, $this->syntax_keys)) { - $this->lint($name, $stanza); - } - } - } - - private function lint($name, $stanza) { - array_push($this->_path, $name); - $this->_checked += 1; - if (is_array($stanza)) { - $this->checkForOldstyle($stanza); - $this->checkEnabled($stanza); - $this->checkUsers($stanza); - $this->checkGroups($stanza); - $this->checkAdmin($stanza); - $this->checkInternal($stanza); - $this->checkPublicURLOverride($stanza); - $this->checkBucketing($stanza); - $this->lintNested($stanza); - } else { - $this->assert(is_string($stanza), "Bad stanza: $stanza."); - } - array_pop($this->_path); - } - - private function assert($ok, $message) { - if (!$ok) { - $loc = "[" . implode('.', $this->_path) . "]"; - array_push($this->_errors, "$loc $message"); - } - } - - private function checkForOldstyle($stanza) { - $enabled = Feature_Util::arrayGet($stanza, Feature_Config::ENABLED, 0); - $rampup = Feature_Util::arrayGet($stanza, 'rampup', null); - $this->assert($enabled !== 'rampup' || !$rampup, "Old-style config syntax detected."); - } - - // 'enabled' must be a string, a number in [0,100], or an array of - // (string => ints) such that the ints are all in [0,100] and the - // total is <= 100. - private function checkEnabled($stanza) { - if (array_key_exists(Feature_Config::ENABLED, $stanza)) { - $enabled = $stanza[Feature_Config::ENABLED]; - if (is_numeric($enabled)) { - $this->assert($enabled >= 0, Feature_Config::ENABLED . " too small: $enabled"); - $this->assert($enabled <= 100, Feature_Config::ENABLED . "too big: $enabled"); - } else if (is_array($enabled)) { - $tot = 0; - foreach ($enabled as $k => $v) { - $this->assert(is_string($k), "Bad key $k in $enabled"); - $this->assert(is_numeric($v), "Bad value $v for $k in $enabled"); - $this->assert($v >= 0, "Bad value $v (too small) for $k"); - $this->assert($v <= 100, "Bad value $v (too big) for $k"); - if (is_numeric($v)) { - $tot += $v; - } - } - $this->assert($tot >= 0, "Bad total $tot (too small)"); - $this->assert($tot <= 100, "Bad total $tot (too big)"); - } - } - } - - private function checkUsers($stanza) { - if (array_key_exists(Feature_Config::USERS, $stanza)) { - $users = $stanza[Feature_Config::USERS]; - if (is_array($users) && !self::isList($users)) { - foreach ($users as $variant => $value) { - $this->assert(is_string($variant), "User variant names must be strings."); - $this->checkUserValue($value); - } - } else { - $this->checkUserValue($users); - } - } - } - - private function checkUserValue($users) { - $this->assert(is_string($users) || self::isList($users), Feature_Config::USERS . " must be string or list of strings: '$users'"); - if (self::isList($users)) { - foreach ($users as $user) { - $this->assert(is_string($user), Feature_Config::USERS . " elements must be strings: '$user'"); - } - } - } - - private function checkGroups($stanza) { - if (array_key_exists(Feature_Config::GROUPS, $stanza)) { - $groups = $stanza[Feature_Config::GROUPS]; - if (is_array($groups) && !self::isList($groups)) { - foreach ($groups as $variant => $value) { - $this->assert(is_string($variant), "Group variant names must be strings."); - $this->checkGroupValue($value); - } - } else { - $this->checkGroupValue($groups); - } - } - } - - private function checkGroupValue($groups) { - $this->assert(is_numeric($groups) || self::isList($groups), Feature_Config::GROUPS . " must be number or list of numbers"); - if (self::isList($groups)) { - foreach ($groups as $group) { - $this->assert(is_numeric($group), Feature_Config::GROUPS . " elements must be numbers: '$group'"); - } - } - } - - - private function checkAdmin($stanza) { - if (array_key_exists(Feature_Config::ADMIN, $stanza)) { - $admin = $stanza[Feature_Config::ADMIN]; - $this->assert(is_string($admin), "Admin must be string naming variant: '$admin'"); - } - } - - private function checkInternal($stanza) { - if (array_key_exists(Feature_Config::INTERNAL, $stanza)) { - $internal = $stanza[Feature_Config::INTERNAL]; - $this->assert(is_string($internal), "Internal must be string naming variant: '$internal'"); - } - } - - private function checkPublicURLOverride($stanza) { - if (array_key_exists(Feature_Config::PUBLIC_URL_OVERRIDE, $stanza)) { - $public_url_override = $stanza[Feature_Config::PUBLIC_URL_OVERRIDE]; - $this->assert(is_bool($public_url_override), "public_url_override must be a boolean: '$public_url_override'"); - if (is_bool($public_url_override)) { - $this->assert($public_url_override === true, "Gratuitous public_url_override (defaults to false)"); - } - } - } - - private function checkBucketing($stanza) { - if (array_key_exists(Feature_Config::BUCKETING, $stanza)) { - $bucketing = $stanza[Feature_Config::BUCKETING]; - $this->assert(is_string($bucketing), "Non-string bucketing: '$bucketing'"); - $this->assert(in_array($bucketing, $this->_legal_bucketing_values), "Illegal bucketing: '$bucketing'"); - } - } - - private static function isList($a) { - return is_array($a) and array_keys($a) === range(0, count($a) - 1); - } -} diff --git a/Feature/Logger.php b/Feature/Logger.php deleted file mode 100644 index 97d1690..0000000 --- a/Feature/Logger.php +++ /dev/null @@ -1,17 +0,0 @@ -_logger = $logger; - } - - /* - * Get the config value for the given key. - */ - public function configValue($name, $default = null) { - return $default; // IMPLEMENT FOR YOUR CONTEXT - } - - /** - * UAID of the current request. - */ - public function uaid() { - return null; // IMPLEMENT FOR YOUR CONTEXT - } - - /** - * User ID of the currently logged in user or null. - */ - public function userID () { - return null; // IMPLEMENT FOR YOUR CONTEXT - } - - /** - * Login name of the currently logged in user or null. Needs the - * ORM. If we're running as part of an Atlas request we ignore the - * passed in userID and return instead the Atlas user name. - */ - public function userName ($userID) { - return null; // IMPLEMENT FOR YOUR CONTEXT - } - - /** - * Is the given user a member of the given group? (This currently, - * like the old config system, uses numeric group IDs in the - * config file, in order to speed up the lookup--the numeric ID is - * the primary key and we save having to look up the group by - * name.) - */ - public function inGroup ($userID, $groupID) { - return null; // IMPLEMENT FOR YOUR CONTEXT - } - - /** - * Is the current user an admin? - * - * @param $userID the id of the relevant user, either the - * currently logged in user or some other user. - */ - public function isAdmin ($userID) { - return false; // IMPLEMENT FOR YOUR CONTEXT - } - - /** - * Is this an internal request? - */ - public function isInternalRequest () { - return false; // IMPLEMENT FOR YOUR CONTEXT - } - - /* - * 'features' query param for url overrides. - */ - public function urlFeatures () { - return array_key_exists('features', $_GET) ? $_GET['features'] : ''; - } - - /* - * Produce a random number in [0, 1) for RANDOM bucketing. - */ - public function random () { - return mt_rand(0, mt_getrandmax() - 1) / mt_getrandmax(); - } - - /* - * Produce a randomish number in [0, 1) based on the given id. - */ - public function hash ($id) { - return self::mapHex(hash('sha256', $id)); - } - - /* - * Record that $variant has been selected for feature named $name - * by $selector and pass the same information along to the logger. - */ - public function log ($name, $variant, $selector) { - $this->_selections[] = array($name, $variant, $selector); - $this->_logger->log($name, $variant, $selector); - } - - /* - * Get the list of selections that we have recorded. The public - * API for getting at the selections is Feature::selections which - * should be the only caller of this method. - */ - public function selections () { - return $this->_selections; - } - - /** - * Map a hex value to the half-open interval [0, 1) while - * preserving uniformity of the input distribution. - * - * @param string $hex a hex string - * @return float - */ - private static function mapHex($hex) { - $len = min(40, strlen($hex)); - $vMax = 1 << $len; - $v = 0; - for ($i = 0; $i < $len; $i++) { - $bit = hexdec($hex[$i]) < 8 ? 0 : 1; - $v = ($v << 1) + $bit; - } - $w = $v / $vMax; - return $w; - } -} diff --git a/Feature/World/Mobile.php b/Feature/World/Mobile.php deleted file mode 100644 index 557f891..0000000 --- a/Feature/World/Mobile.php +++ /dev/null @@ -1,56 +0,0 @@ -_udid = $udid; - $this->_userID = $userID; - } - - public function uaid() { - return $this->_udid; - } - - public function userID () { - return $this->_userID; - } - - public function log ($name, $variant, $selector) { - parent::log($name, $variant, $selector); - - $this->_name = $name; - $this->_variant = $variant; - $this->_selector = $selector; - } - - public function getLastName() { - return $this->_name; - } - - public function getLastVariant() { - return $this->_variant; - } - - public function getLastSelector() { - return $this->_selector; - } - - public function clearLastFeature() { - $this->_selector = null; - $this->_name = null; - $this->_variant = null; - } - -} diff --git a/GENERALIZING.md b/GENERALIZING.md deleted file mode 100644 index 4dd0cf0..0000000 --- a/GENERALIZING.md +++ /dev/null @@ -1,101 +0,0 @@ -# A theory about generalizing this code - -This code was written at Etsy to meet our specific needs and with a -strong goal of making the code as simple as possible to understand. -Which means that there are places where stuff is hardwired because we -didn’t need the flexibility to do things another way. - -Obviously, some of the concepts embedded in this code are not going to -be applicable outside the Etsy context. Thus if you want to use this -code in a different context you have two choices: fork and hack or -generalize. I’d actually suggest you start with the former. Hopefully -things are structured well enough that if you rip out the -Etsy-specific code you’ll be left with a few obvious holes to fill in -with your own stuff. I’ve started things down that path for you by -turning several methods into no-ops and marking with the comment -“IMPLEMENT FOR YOUR CONTEXT”. - -However even with those bits ripped out, the structure of things in -`master` is still tied to its Etsy heritage so, I've also made a quick -start at that in the `generalized` branch. Note that the code in this -branch is completely untested and may still be wrongheaded in many -ways. (There are plans at Etsy to finish up this work and the port it -back into our own codebase.) - -The basic approach I took in that branch was to introduce a new -abstraction, the "experimental unit". Every feature is tested relative -to some kind of experimental unit which is named in the feature's -configuration (under the `unit` key) though the Feature_World -implementation can provide a default. Each kind of experimental unit -can support: - -- explicit configuration of variants based on some characteristic of - the unit. - -- different bucketing schemes. - -As an example, the `Feature_EtsyRequestUnit` class, implements an -experimental unit that maps to a web request. Each web request (in the -Etsy context) has some information about the user who made the request -(at least a cookie called the UAID and possibly an Etsy user ID if -they are signed in). Additionally the request itself may have included -a `features` query param that specifies specific variants for specific -features and may also be an "internal" request, coming from someone -within Etsy. - -The configuration syntax for a feature configured with this -experimental unit (which is the default in the current implementation -of `Feature_World`) can be configured with `users`, `groups`, `admin`, -and `internal` keys, that specify variants to be assigned to specific -users, users in specific groups, all Etsy employees (called "admin"), -or for internal requests. - -This experimental unit also supports three bucketing styles: 'uaid', -'user', and 'random'. The 'uaid' style uses the cookie that is set on -every request as the bucketing ID while the 'user' style uses the user -ID of signed in users. Random bucketing, which assigns a variant -randomly on each request, is only used for operational ramupus without -user-visible effects such as switching from one backend database to -another. - -When a call is made to `Feature::isEnabled` or `Feature::variant`, the -experimental unit is responsible for saying whether a specific variant -should be used (via the `assignedVariant` method) and, if not, what id -should be used for bucketing the experimental unit into a variant (via -`bucketingID`). - -In the generalized branch, both those methods take a second argument, -`$data`, which is passed along to the `assignedVariant` and -`bucketingID` methods. In general, implementations of these methods -should ensure that any data they are passed is of the appropriate -type: there is an obligation on callers of the Feature API methods to -pass the appropriate kind of date for the kind of experimental unit -the feature has been configured with. - -One thing this generalization does is get rid of the need for the -`isEnabledFor`/`variantFor` and -`isEnabledBucketingBy`/`variantBucketingBy` methods. The main use -case, at Etsy, for the former pair is if we wanted to run an -experiment where insted of bucketing by the user making a request, we -want to bucket by the user who owns the shop the user is looking at. -In the current API, that is achieved by passing the user object -representing the shop owner to `isEnabledFor` and `variantFor`. And -the use case for the `bucketingBy` methods is when we want to bucket -on something that doesn't necessarily have an associated user. For -instance if we wanted to run an experiment on a random selection of -searches, we might use the search terms as the second argument to the -`bucketingBy` methods. - -In the generalized API, in the first case we would instead configure a -feature with a `unit` of, say, 'seller' that would map to a class that -either expects some object reperesenting the seller to be passed to -the Feature API calls or which knows how to figure out the seller from -the context of the request. And in the second case we would configure -a query with a `unit` of 'query' that maps to a class that expects a -query string passed as the `$data` argument to the Feature methods. -With such a class, we could then allow the feature to be configured to -return a specific variant for specific queries, e.g. if we want to -ensure that certain very popular queries are kept out of the treatment -group for whatever reason. - --Peter Seibel \ No newline at end of file diff --git a/LICENSE b/LICENSE deleted file mode 100644 index 91d077d..0000000 --- a/LICENSE +++ /dev/null @@ -1,22 +0,0 @@ - Copyright (c) 2010 Etsy - - Permission is hereby granted, free of charge, to any person - obtaining a copy of this software and associated documentation - files (the "Software"), to deal in the Software without - restriction, including without limitation the rights to use, - copy, modify, merge, publish, distribute, sublicense, and/or sell - copies of the Software, and to permit persons to whom the - Software is furnished to do so, subject to the following - conditions: - - The above copyright notice and this permission notice shall be - included in all copies or substantial portions of the Software. - - THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, - EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES - OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND - NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT - HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, - WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR - OTHER DEALINGS IN THE SOFTWARE. diff --git a/README.md b/README.md index b4a4bf8..6e85247 100644 --- a/README.md +++ b/README.md @@ -1,544 +1,228 @@ -Feature is no longer actively maintained and is no longer in sync with the version used internally at Etsy. +[![GitHub license](https://img.shields.io/github/license/PabloJoan/feature.svg)](https://github.com/PabloJoan/feature/blob/master/LICENSE) -# Feature API - -Etsy's Feature flagging API used for operational rampups and A/B -testing. - -The Feature API is how we selectively enable and disable features at a -very fine grain as well as enabling features for a percentage of users -for operational ramp-ups and for A/B tests. A feature can be -completely enabled, completely disabled, or something in between and -can comprise a number of related variants. +Requires PHP 8.3 and above. -For features that are not completely enabled or disabled, we log every -time we check whether a feature is enabled and include the result, -including what variant was selected, in the events we fire. +# Installation -The two main API entry points are: +```bash +composer require pablojoan/feature +``` - Feature::isEnabled('my_feature') +# Basic Usage -which returns true when `my_feature` is enabled and, for multi-variant -features: +```php - Feature::variant('my_feature') +use PabloJoan\Feature\Features; // Import the namespace. -which returns the name of the particular variant which should be used. +$featureConfigs = [ + 'foo' => [ + 'variants' => [ + 'variant1' => 100, //100% chance this variable will be chosen + 'variant2' => 0 //0% chance this variable will be chosen + ] + ], + 'bar' => [ + 'variants' => [ + 'variant1' => 25, //25% chance this variable will be chosen + 'variant2' => 25, //25% chance this variable will be chosen + 'variant3' => 50 //50% chance this variable will be chosen + ], + 'bucketing' => 'id' //same id string will always return the same variant + ] +]; -The single argument to each of these methods is the name of the -feature to test. +$features = new Features($featureConfigs); -A typical use of `Feature::isEnabled` for a single-variant feature -would look something like this: +$features->isEnabled(featureName: 'foo'); // true +$features->getEnabledVariant(featureName: 'foo'); // 'variant1' +``` - if (Feature::isEnabled('my_feature')) { - // do stuff - } +For a quick summary and common use cases, please read the rest of this README. -For a multi-variant feature, within the block guarded by the -`Feature::isEnabled` check, we can determine the appropriate code to -run for each variant with something like this: +# Feature API - if (Feature::isEnabled('my_feature')) { +Feature flagging API used for operational rampups and A/B testing. - switch (Feature::variant('my_feature')) { - case 'foo': - // do stuff appropriate for the foo variant - break; - case 'bar': - // do stuff appropriate for the bar variant - break; - } - } +The Feature API is how we selectively enable and disable features at a very fine +grain as well as enabling features for a percentage of users for operational +ramp-ups and for A/B tests. +A feature can be completely enabled, completely disabled, or something in +between and can comprise a number of related variants. -It is an error (and will be logged as such) to ask for the variant of -a feature that is not enabled. So the calls to variant should always -be guarded by an `Feature::isEnabled` check. +The two main API entry points are: -The API also provides two other pairs of methods that will be used -much less frequently: +```php + $features->isEnabled(featureName: 'my_feature') +``` - Feature::isEnabledFor('my_feature', $user) +which returns true when `my_feature` is enabled and, for multi-variant features: - Feature::variantFor('my_feature', $user) +```php + $features->getEnabledVariant(featureName: 'my_feature') +``` -and +which returns the name of the particular variant which should be used. - Feature::isEnabledBucketingBy('my_feature', $bucketingID) +The single argument to each of these methods is the name of the +feature to test. - Feature::variantBucketingBy('my_feature', $bucketingID) +A typical use of `$features->isEnabled` for a single-variant feature +would look something like this: -These methods exist only to support a couple very specific use-cases: -when we want to enable or disable a feature based not on the user -making the request but on some other user or when we want to bucket a -percentage of executions based on something entirely other than a -user.) The canonical case for the former, at Etsy, is if we wanted to -change something about how we deal with listings and instead of -enabling the feature for only some users but for all listings those -users see, but instead we want to enable it for all users but for only -some of the listings. Then we could use `isEnabledFor` and -`variantFor` and pass in the user object representing the owner of the -listing. That would also allow us to enable the feature for specific -listing owners. The `bucketingBy` methods serve a similar purpose -except when there either is no relevant user or where we don't want to -always put the same user in the same bucket. Thus if we wanted to -enable a certain feature for 10% of all listings displayed, -independent of both the user making the request and the user who owned -the listing, we could use `isEnabledBucketingBy` with the listing id -as the bucketing ID. +```php + if ($features->isEnabled(featureName: 'my_feature')) { + // do stuff + } +``` + +For a multi-variant feature, we can determine the appropriate code to run for +each variant with something like this: + +```php + switch ($features->getEnabledVariant(featureName: 'my_feature')) { + case 'foo': + // do stuff appropriate for the 'foo' variant + break; + case 'bar': + // do stuff appropriate for the 'bar' variant + break; + } +``` -In general it is much more likely you want to use the plain old -`isEnabled` and `variant` methods. +If a feature is bucketed by id, then we pass the id string to +`$features->isEnabled` and `$features->getEnabledVariant` as a second parameter -For Smarty templates, where static methods can’t readily be called, -there is an object, `$feature`, wired up in Tpl.php that exposes the -same four methods as the Feature API but as instance methods, for -instance: +```php + $isMyFeatureEnabled = $features->isEnabled( + featureName: 'my_feature', + id: 'unique_id_string' + ); - {% if $feature->isEnabled("my_feature") %} + $variant = $features->getEnabledVariant( + featureName: 'my_feature', + id: 'unique_id_string' + ); +``` ## Configuration cookbook -There are a number of common configurations so before I explain the -complete syntax of the feature configuration stanzas, here are some of -the more common cases along with the most concise way to write the -configuration. +There are a number of common configurations so before I explain the complete +syntax of the feature configuration stanzas, here are some of the more common +cases along with the most concise way to write the configuration. ### A totally enabled feature: - $server_config['foo'] = 'on'; +```php + $server_config['foo'] = ['variants' => ['enabled' => 100]]; +``` ### A totally disabled feature: - $server_config['foo'] = 'off'; +```php + $server_config['foo'] = ['variants' => ['enabled' => 0]]; +``` ### Feature with winning variant turned on for everyone - $server_config['foo'] = 'blue_background'; - -### Feature enabled only for admins: - - $server_config['foo'] = array('admin' => 'on'); +```php + $server_config['foo'] = ['variants' => ['blue_background' => 100]]; +``` ### Single-variant feature ramped up to 1% of users. - $server_config['foo'] = array('enabled' => 1); +```php + $server_config['foo'] = ['variants' => ['enabled' => 1]]; +``` ### Multi-variant feature ramped up to 1% of users for each variant. - $server_config['foo'] = array( - 'enabled' => array( +```php + $server_config['foo'] = [ + 'variants' => [ 'blue_background' => 1, 'orange_background' => 1, 'pink_background' => 1, - ), - ); - -### Enabled for a single specific user. - - $server_config['foo'] = array('users' => 'fred'); - -### Enabled for a few specific users. - - $server_config['foo'] = array( - 'users' => array('fred', 'barney', 'wilma', 'betty'), - ); + ], + ]; +``` -### Enabled for a specific group +### Enabled for 10% of regular users. - $server_config['foo'] = array('groups' => 1234); +```php + $server_config['foo'] = [ + 'variants' => ['enabled' => 10] + ]; +``` -### Enabled for 10% of regular users and all admin. +### Feature ramped up to 1% of requests, bucketing at random rather than by id - $server_config['foo'] = array( - 'enabled' => 10, - 'admin' => 'on', - ); +```php + $server_config['foo'] = [ + 'variants' => ['enabled' => 1], + 'bucketing' => 'random' + ]; +``` -### Feature ramped up to 1% of requests, bucketing at random rather than by user +### Feature ramped up to 40% of requests, bucketing by id rather than at random - $server_config['foo'] = array( - 'enabled' => 1, - 'bucketing' => 'random', - ); +```php + $server_config['foo'] = [ + 'variants' => ['enabled' => 40], + 'bucketing' => 'id' + ]; +``` ### Single-variant feature in 50/50 A/B test - $server_config['foo'] = array('enabled' => 50); +```php + $server_config['foo'] = ['variants' => ['enabled' => 50]]; +``` ### Multi-variant feature in A/B test with 20% of users seeing each variant (and 40% left in control group). - $server_config['foo'] = array( - 'enabled' => array( +```php + $server_config['foo'] = [ + 'variants' => [ 'blue_background' => 20, 'orange_background' => 20, - 'pink_background' => 20, - ), - ); - -### New feature intended only to be enabled by adding ?features=foo to a URL - - $server_config['foo'] = array('enabled' => 0); - -This is kind of a funny edge case. It could also be written: - - $server_config['foo'] = array(); - -since a missing `'enabled'` is defaulted to 0. + 'pink_background' => 20 + ], + ]; +``` ## Configuration details -Each feature’s config stanza controls when the feature is enabled and -what variant should be used when it is. - -Leaving aside a few shorthands that will be explained in a moment, the -value of a feature config stanza is an array with a number of special -keys, the most important of which is `'enabled'`. - -In its full form, the value of the `'enabled'` property is either the -string `'off'`, meaning the feature is entirely disabled, any other -string, meaning the named variant is enabled for all requests, or an -array whose keys are names of variants and whose values are the -percentage of requests that should see each variant. - -As a shorthand to support the common case of a feature with only one -variant, `'enabled'` can also be specified as a percentage from 0 to -100 which is equivalent to specifying an array with the variant name -`'on'` and the given percentage. - -The next four most important properties of a feature config stanza -specify a particular variant that special classes of users should see: -`'admin'`, `'internal'`, `'users'`, and `'groups'`. - -The `'admin'` and `'internal'` properties, if present, should name a -variant that should be shown for all admin users or all internal -requests. For single-variant features this name will almost always be -`'on'`. (Technically you could also specify `'off'` to turn off a -feature for admin users or internal requests that would be otherwise -enabled. But that would be weird.) For multi-variant features it can -be any of the variants mentioned in the `'enabled'` array. - -The `'users'` and `'groups'` variants provide a mapping from variant -names to lists of users or numeric group ids. In the fully specified -case, the value will be an array whose keys are the names of variants -and whose values are lists of user names or group ids, as appropriate. -As a shorthand, if the list of user names or group ids is a single -element it can be specified with just the name or id. And as a further -shorthand, in the configuration of a single-variant feature, the value -of the `'users'` or `'groups'` property can simply be the value that -should be assigned to the `'on'` variant. So using both shorthands, -these are equivalent: - - $server_config['foo'] => array('users' => array('on' => array('fred'))); - -and: - - $server_config['foo'] => array('users' => 'fred'); - -None of these four properties have any effect if `'enabled'` is a -string since in those cases the feature is considered either entirely -enabled or disabled. They can, however, enable a variant of a feature -if no `'enabled'` value is provided or if the variant’s percentage is -0. - -On the other hand, when an array `'enabled'` value is specified, as an -aid to detecting typos, the variant names used in the `'admin'`, -`'internal'`, `'users'`, and `'groups'` properties must also be keys -in the `'enabled'` array. So if any variants are specified via -`'enabled'`, they should all be, even if their percentage is set to 0. - -The two remaining feature config properties are `'bucketing'` and -`'public_url_override'`. Bucketing specifies how users are bucketed -when a feature is enabled for only a percentage of users. The default -value, `'uaid'`, causes bucketing via the UAID cookie which means a -user will be in the same bucket regardless of whether they are signed -in or not. - -The bucketing value `'user'`, causes bucketing to be based on the -signed-in user id. Currently we fall back to bucketing by UAID if the -user is not signed in but this is problematic since it means that a -user can switch buckets if they sign in or out. (We may change the -behavior of this bucketing scheme to simply disable the feature for -users who are not signed in.) - -Finally the bucketing value `'random'`, causes each request to be -bucketed independently meaning that the same user will be in different -buckets on different requests. This is typically used for features -that should have no user-visible effects but where we want to ramp up -something like the switch from master to shards or a new version of -jquery. - -The `'public_url_override'` property allows all requests, not just -admin and internal requests, to turn on a feature and choose a variant -via the `features` query param. Its value will almost always be true -if it is present since it defaults to false if omitted. - -Finally, two last shorthands: - -First, a config stanza with only the key `'enabled'` and a string -value can be replaced with just the string. So: - - $server_config['foo'] = array('enabled' => 'on'); - $server_config['bar'] = array('enabled' => 'off'); - $server_config['baz'] = array('enabled' => 'some_variant'); - -Can be written simply: - - $server_config['foo'] = 'on'; - $server_config['bar'] = 'off'; - $server_config['baz'] = 'some_variant'; - -And second, if a feature config is missing entirely, it’s equivalent -to specifying it as `'off'`. This allows dark changes to include code -that checks for a feature before it has been added to production.php. - -**Note for ops**: removing a feature config altogether, setting it to -the string `'off'`, or setting `'enabled'` to `'off'` all completely -disable the feature, ensuring that code guarded by -`Feature::isEnabled` for that feature will never run. The best way to -turn off an existing feature in an emergency would be to set -`'enabled'` to `'off'`. To facilitate that, we should try to keep the -`'enabled'` value on one line, whenever possible. Thus: - - $server_config['foo'] = array( - 'enabled' => array('foo' => 10, 'bar' => 10), - ); - -rather than - - $server_config['foo'] = array( - 'enabled' => array( - 'foo' => 10, - 'bar' => 10 - ), - ); - -so that the bleary-eyed, junior ops person at 3am can do this: - - $server_config['foo'] = array( - 'enabled' => 'off', // array('foo' => 10, 'bar' => 10), - ); - -rather than this, which breaks the config file: - - $server_config['foo'] = array( - 'enabled' => 'off', // array( - 'foo' => 10, - 'bar' => 10 - ), - ); - -Note, however, that removing the `'enabled'` property does mostly turn -off the feature it doesn’t completely disable it as it could still be -enabled via an `'admin'` property, etc. - -## Precedence: - -The precedence of the various mechanisms for enabling a feature are as -follows. - - - If `'enabled'` is a string (variant name or `'off'`) the feature - is entirely on or off for all requests. - - - Otherwise, if the request is from an admin user or is an internal - request, or if `'public_url_override'` is true and the request - contains a `features` query param that specifies a variant for the - feature in question, that variant is used. The value of the - `features` param is a comma-delimited list of features where each - feature is either simply the name of the feature, indicating the - feature should be enabled with variant `'on'` or the name of a - feature, a colon, and the variant name. E.g. a request with - `features=foo,bar:x,baz:off` would turn on feature `foo`, turn on - feature `bar` with variant `x`, and turn off feature `baz`. - - - Otherwise, if the request is from a user specified in the - `'users'` property, the specified variant is enabled. +Each feature’s config stanza controls when the feature is enabled and what +variant should be used when it is. - - Otherwise, if the request is from a member of a group specified in - the `'groups'` property the specified variant is enabled. (The - behavior when the user is a member of multiple groups that have - been assigned different variants is undefined. Beware nasal - demons.) +The value of a feature config stanza is an array with a number of special +keys, the most important of which is `'variants'`. - - Otherwise, if the request is from an admin, the `'admin'` variant - is enabled. +The value of the `'variants'` property an array whose keys are names of variants +and whose values are the percentage of requests that should see each variant. - - Otherwise, if the request is an internal request, the `'internal'` - variant is enabled. +The remaining feature config property is `'bucketing'`. Bucketing specifies +how users are bucketed when a feature is enabled for only a percentage of users. +The default value, `'random'`, causes each request to be bucketed independently, +meaning that the same user will be in different buckets on different requests. +This is typically used for features that should have no user-visible effects +but where we want to ramp up something like the switch from master to shards +or a new version of JS code. - - Otherwise, the request is bucketed and a variant is chosen so that - the correct percentage of bucketed requests will see each variant. +The bucketing value `'id'`, causes bucketing to be based on the given id. ## Errors -There are a few ways to misuse the Feature API or misconfigure a -feature that may be detected and logged. (Some of these are not -currently detected but may be in the future.) - - 1. Calling `Feature::variant` for a single-variant feature. - - 1. Calling `Feature::variant` in code not guarded by an - `Feature::isEnabled` check. - - 1. Including `'on'` as a variant name in a multi-variant feature. - - 1. Setting `'enabled'` to numeric value less than 0 or greater than - 100. - - 1. Setting the percentage value of a variant in `'enabled'` to a - value less than 0 or greater than 100. - - 1. Setting `'enabled'` such that the sum of the variant percentages - is greater than 100. - - 1. Setting `'enabled'` to a non-numeric, non-string, non-array - value. - - 1. When `'enabled'` is an array, setting the `'users'` or `'groups'` - property to an array that includes a key that is not a key in - `'enabled'`. - - 1. When `'enabled'` is an array, setting the `'admin'` or - `'internal'` property to a value that is not a key in `'enabled'`. - -## The life cycle of a feature - -The Feature API was designed with a eye toward making it a bit easier -for us to push features through a predictable life cycle wherein a -feature can be created easily, ramped up, A/B tested, and then cleaned -up, either by being promoted to a full-fledged feature flag, by -removing the configuration and associated feature checks but keeping -the code, or deleting the code altogether. - -The basic life cycle of a feature might look like this: - - 1. Developer writes some code guarded by `Feature::isEnabled` - checks. In order to test the feature in development they will add - configuration for the feature to `development.php` that turns it - on for specific users or admin or sets `'enabled'` to 0 so they - can test it with a URL query param. - - 1. At some point the developer will add a config stanza to - `production.php`. Initially this may just be a place holder that - leaves the feature entirely disabled or it may turn it on for - admin, etc. - - 1. Once the feature is done, the `production.php` config will be - changed to enable the feature for a small percentage of users for - an operational smoke test. For a single-variant feature this means - setting `'enabled'` to a small numeric value; for a multi-variant - feature it means setting `'enabled'` to an array that specifies a - small percentage for each variant. - - 1. During the rampup period the percentage of users exposed to the - feature may be moved up and down until the developers and ops - folks are convinced the code is fully baked. If serious problems - arise at any point, the new code can be completely disabled by - setting enabled to `'off'`. - - 1. If the feature is going to be part of an A/B experiment, then the - developers will (working with the data team) figure out the best - percentage of users to expose the feature to and how long the - experiment will have to run in order to gather good experimental - data. To launch the experiment the production config will be - changed to enable the feature or its variants for the appropriate - percentage of users. After this point the percentages should be - left alone until the experiment is complete. - -At this point there are a number of things that can happen: if the -experiment revealed a clear winner we may simply want to keep the -code, possibly putting it under control of a top-level feature flag -that ops can use to disable the feature for operational reasons. Or we -may want to discard all the code related to the feature. Or we may -want to run another experiment based on what we learned from this one. -Here’s what will happen in those cases: - -### To keep the feature as a permanent part of the web site without creating a top-level feature flag - - 1. Change the value of the feature config to the name of the winning - variant (`'on'` for a single-variant feature). - - 1. Delete any code that implements other variants and remove the - calls to `Feature::variant` and any related conditional logic - (e.g. switches on the variant name). - - 1. Remove the `Feature::isEnabled` checks but keep the code they - guarded. - - 1. Remove the feature config. - -### To keep a feature under the control of a full-fledged feature flag. (I.e. for things that will typically be enabled but which we want to preserve the ability to turn off with a simple config change.) - - 1. Change the value of the feature config to the name of the winning - variant (`'on'` for a single-variant feature). - - 1. Delete any code that implements other variants and remove the - calls to `Feature::variant` and any related conditional logic - (e.g. switches on the variant name). - - 1. Add a new config named with a `feature_` prefix and set its value - to `'on'`. - - 1. Change all the `Feature::isEnabled` checks for the old flag name - to the new feature flag. - - 1. Remove the old config. - -### To remove a feature all together - - 1. Change the value of the feature config to `'off'`. - - 1. Delete all code guarded by `Feature::isEnabled` checks and then - remove the checks. - - 1. Remove the feature config. - -### To run a new experiment based on the same code - - 1. Set the enabled value of the feature config to `'off'`. - - 1. Create a new feature config with a similar name but suffixed with - _vN where N is 2 if this is the second experiment, 3 if is the - third. Set it to `'off'`. - - 1. Change all the `Feature::isEnabled` checks for the old feature to - the new feature. - - 1. Delete the old config. - - 1. Implement the changes required for the new experiment, deleting - old variants and adding new ones as needed. - - 1. Rampup and then A/B test the new feature as normal. - - 1. Promote, cleanup, or re-experiment as appropriate. - -## A few style guidelines +There are a few ways to misuse the Feature API or misconfigure a feature that +may be detected. (Some of these are not currently detected but may be in the +future.) -To make it easier to push features through this life cycle there are a -few coding guidelines to observe. +1. Setting the percentage value of a variant in `'variants'` to a value less + than 0 or greater than 100. -First, the feature name argument to the Feature methods (`isEnabled`, -`variant`, `isEnabledFor`, and `variantFor`) should always be a string -literal. This will make it easier to find all the places that a -particular feature is checked. If you find yourself creating feature -names at run time and then checking them, you’re probably abusing the -Feature system. Chances are in such a case you don’t really want to be -using the Feature API but rather simply driving your code with some -plain old config data. +2. Setting `'variants'` such that the sum of the variant percentages is + greater than 100. -Second, the results of the Feature methods should not be cached, such -as by calling `Feature::isEnabled` once and storing the result in an -instance variable of some controller. The Feature machinery already -caches the results of the computation it does so it should already be -plenty fast to simply call `Feature::isEnabled` or `Feature::variant` -whenever needed. This will again aid in finding the places that depend -on a particular feature. +3. Setting `'variants'` to a non-array value. -Third, as a check that you’re using the Feature API properly, whenever -you have an if block whose test is a call to `Feature::isEnabled`, -make sure that it would make sense to either remove the check and keep -the code or to delete the check and the code together. There shouldn’t -be bits of code within a block guarded by an isEnabled check that -needs to be salvaged if the feature is removed. +4. Setting `'bucketing'` to any value that is not `'id'` or `'random'`. diff --git a/UNLICENSE b/UNLICENSE new file mode 100644 index 0000000..68a49da --- /dev/null +++ b/UNLICENSE @@ -0,0 +1,24 @@ +This is free and unencumbered software released into the public domain. + +Anyone is free to copy, modify, publish, use, compile, sell, or +distribute this software, either in source code form or as a compiled +binary, for any purpose, commercial or non-commercial, and by any +means. + +In jurisdictions that recognize copyright laws, the author or authors +of this software dedicate any and all copyright interest in the +software to the public domain. We make this dedication for the benefit +of the public at large and to the detriment of our heirs and +successors. We intend this dedication to be an overt act of +relinquishment in perpetuity of all present and future rights to this +software under copyright law. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR +OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +OTHER DEALINGS IN THE SOFTWARE. + +For more information, please refer to diff --git a/composer.json b/composer.json new file mode 100644 index 0000000..b1b5824 --- /dev/null +++ b/composer.json @@ -0,0 +1,43 @@ +{ + "name": "pablojoan/feature", + "description": "Feature flagging API used for operational rampups and A/B testing", + "authors": [ + { + "name": "Pablo Iglesias", + "email": "iglesias.pablo10@gmail.com" + } + ], + "license": "Unlicense", + "keywords": [ + "feature-flags", + "ab-testing", + "ab-test", + "feature", + "feature-flagging", + "feature-toggles", + "feature-toggle", + "ab-tests", + "a-b-testing", + "a-b-test", + "toggle" + ], + "require": { + "php": ">=8.3" + }, + "require-dev": { + "phpunit/phpunit": "*" + }, + "scripts": { + "test": "php ./vendor/bin/phpunit --stop-on-failure --fail-on-warning --fail-on-risky tests/" + }, + "autoload": { + "psr-4": { + "PabloJoan\\Feature\\": "src/" + } + }, + "autoload-dev": { + "psr-4": { + "PabloJoan\\Feature\\Tests\\": "tests/" + } + } +} diff --git a/phpunit/Feature/ConfigTest.php b/phpunit/Feature/ConfigTest.php deleted file mode 100644 index 94ea7d1..0000000 --- a/phpunit/Feature/ConfigTest.php +++ /dev/null @@ -1,351 +0,0 @@ -expectDisabled($c, array('uaid' => 0)); - $this->expectDisabled($c, array('uaid' => 1)); - } - - function testFullyEnabled() { - $c = array('enabled' => 'on'); - $this->expectEnabled($c, array('uaid' => '0')); - $this->expectEnabled($c, array('uaid' => '1')); - } - - function testSimpleDisabled () { - $c = array('enabled' => 'off'); - $this->expectDisabled($c, array('uaid' => '0')); - $this->expectDisabled($c, array('uaid' => '1')); - } - - function testVariantEnabled () { - $c = array('enabled' => 'winner'); - $this->expectEnabled($c, array('uaid' => '0'), 'winner'); - $this->expectEnabled($c, array('uaid' => '1'), 'winner'); - } - - function testFullyEnabledString() { - $c = 'on'; - $this->expectEnabled($c, array('uaid' => '0')); - $this->expectEnabled($c, array('uaid' => '1')); - } - - function testSimpleDisabledString () { - $c = 'off'; - $this->expectDisabled($c, array('uaid' => '0')); - $this->expectDisabled($c, array('uaid' => '1')); - } - - function testVariantEnabledString () { - $c = 'winner'; - $this->expectEnabled($c, array('uaid' => '0'), 'winner'); - $this->expectEnabled($c, array('uaid' => '1'), 'winner'); - } - - function testSimpleRampup () { - $c = array('enabled' => '50'); - $this->expectEnabled($c, array('uaid' => '0')); - $this->expectEnabled($c, array('uaid' => '.1')); - $this->expectEnabled($c, array('uaid' => '.4999')); - $this->expectDisabled($c, array('uaid' => '.5')); - $this->expectDisabled($c, array('uaid' => '.6')); - $this->expectDisabled($c, array('uaid' => '.99')); - $this->expectDisabled($c, array('uaid' => '1')); - } - - function testMultivariant () { - $c = array('enabled' => array('foo' => 2, 'bar' => 3)); - $this->expectEnabled($c, array('uaid' => '0'), 'foo'); - $this->expectEnabled($c, array('uaid' => '.01'), 'foo'); - $this->expectEnabled($c, array('uaid' => '.01999'), 'foo'); - $this->expectEnabled($c, array('uaid' => '.02'), 'bar'); - $this->expectEnabled($c, array('uaid' => '.04999'), 'bar'); - $this->expectDisabled($c, array('uaid' => '.05')); - $this->expectDisabled($c, array('uaid' => '1')); - } - - /* - * Is feature disbaled by enabled => off despite every other - * setting trying to turn it on? - */ - function testComplexDisabled () { - $c = array( - 'enabled' => 'off', - 'users' => array('fred', 'sally'), - 'groups' => array(1234, 2345), - 'admin' => 'on', - 'internal' => 'on', - 'public_url_overrride' => true - ); - - $this->expectDisabled($c, array('isInternal' => true, 'uaid' => '0')); - $this->expectDisabled($c, array('userName' => 'fred', 'uaid' => '0')); - $this->expectDisabled($c, array('inGroup' => array(0 => 1234), 'uaid' => '0')); - $this->expectDisabled($c, array('uaid' => '100', 'uaid' => '0')); - $this->expectDisabled($c, array('isAdmin' => true, 'uaid' => '0')); - $this->expectDisabled($c, array('isInternal' => true, 'urlFeatures' => 'foo', 'uaid' => 0)); - - // Now all at once. - $this->expectDisabled($c, array( - 'isInternal' => true, - 'userName' => 'fred', - 'inGroup' => array(0 => 1234), - 'uaid' => '100', - 'isAdmin' => true, - 'urlFeatures' => 'foo', - 'userID' => '0')); - } - - function testAdminOnly () { - $c = array('enabled' => 0, 'admin' => 'on'); - $this->expectEnabled($c, array('isAdmin' => true, 'uaid' => '0', 'userID' => '1')); - $this->expectDisabled($c, array('isAdmin' => false, 'uaid' => '1', 'userID' => '1')); - } - - function testAdminPlusSome () { - $c = array('enabled' => 10, 'admin' => 'on'); - $this->expectEnabled($c, array('isAdmin' => true, 'uaid' => '.5', 'userID' => '1')); - $this->expectEnabled($c, array('isAdmin' => false, 'uaid' => '.05', 'userID' => '1')); - $this->expectDisabled($c, array('isAdmin' => false, 'uaid' => '.5', 'userID' => '1')); - } - - function testInternalOnly () { - $c = array('enabled' => 0, 'internal' => 'on'); - $this->expectEnabled($c, array('isInternal' => true, 'uaid' => '0')); - $this->expectDisabled($c, array('isInternal' => false, 'uaid' => '1')); - } - - function testInternalPlusSome () { - $c = array('enabled' => 10, 'internal' => 'on'); - $this->expectEnabled($c, array('isInternal' => true, 'uaid' => '.5')); - $this->expectEnabled($c, array('isInternal' => false, 'uaid' => '.05')); - $this->expectDisabled($c, array('isInternal' => false, 'uaid' => '.5')); - } - - function testOneUser () { - $c = array('enabled' => 0, 'users' => 'fred'); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'fred', 'userID' => '1')); - $this->expectDisabled($c, array('uaid' => '1', 'userName' => 'george', 'userID' => '1')); - $this->expectDisabled($c, array('userID' => null, 'uaid' => 0)); - } - - function testListOfOneUser () { - $c = array('enabled' => 0, 'users' => array('fred')); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'fred', 'userID' => '1')); - $this->expectDisabled($c, array('uaid' => '1', 'userName' => 'george', 'userID' => '1')); - } - - function testListOfUsers () { - $c = array('enabled' => 0, 'users' => array('fred', 'ron')); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'fred', 'userID' => '1')); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'ron', 'userID' => '1')); - $this->expectDisabled($c, array('uaid' => '1', 'userName' => 'george', 'userID' => '1')); - } - - function testListOfUsersCaseInsensitive() { - $c = array('enabled' => 0, 'users' => array('fred', 'FunGuy')); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'fred', 'userID' => '1')); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'FunGuy', 'userID' => '1')); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'FUNGUY', 'userID' => '1')); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'funguy', 'userID' => '1')); - } - - function testArrayOfUsers () { - // It might be kind of nice to allow 'enabled' => 0 here but - // then we lose the ability to check that the variants - // mentioned in a users clause are actually valid - // variants. Which maybe is okay: perhaps we'd like to be able - // to enable variants for users that are otherwise disabled. - $c = array('enabled' => array('twins' => 0, 'other' => 0), - 'users' => array( - 'twins' => array('fred', 'george'), - 'other' => 'ron')); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'fred', 'userID' => '1'), 'twins'); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'george', 'userID' => '2'), 'twins'); - $this->expectEnabled($c, array('uaid' => '1', 'userName' => 'ron', 'userID' => '3'), 'other'); - $this->expectDisabled($c, array('uaid' => '0', 'userName' => 'percy', 'userID' => '4')); - } - - function testOneGroup () { - $c = array('enabled' => 0, 'groups' => 1234); - $this->expectEnabled($c, array('uaid' => 1, 'userID' => 1, 'inGroup' => array(1 => array(1234)))); - $this->expectDisabled($c, array('uaid' => 0, 'userID' => 2, 'inGroup' => array(2 => array(2345)))); - $this->expectDisabled($c, array('uaid' => 0, 'userID' => null, 'uaid' => 0)); - } - - function testListOfOneGroup () { - $c = array('enabled' => 0, 'groups' => array(1234)); - $this->expectEnabled($c, array('uaid' => 1, 'userID' => 1, 'inGroup' => array(1 => array(1234)))); - $this->expectDisabled($c, array('uaid' => 0, 'userID' => 2, 'inGroup' => array(2 => array(2345)))); - } - - function testListOfGroups () { - $c = array('enabled' => 0, 'groups' => array(1234, 2345)); - $this->expectEnabled($c, array('uaid' => 1, 'userID' => 1, 'inGroup' => array(1 => array(1234)))); - $this->expectEnabled($c, array('uaid' => 1, 'userID' => 2, 'inGroup' => array(2 => array(2345)))); - $this->expectDisabled($c, array('uaid' => 0, 'userID' => 3, 'inGroup' => array(3 => array()))); - } - function testArrayOfGroups () { - // See comment at testArrayOfUsers; similar issue applies here. - $c = array('enabled' => array('twins' => 0, 'other' => 0), - 'groups' => array( - 'twins' => array(1234, 2345), - 'other' => 3456)); - $this->expectEnabled($c, array('uaid' => 1, 'userID' => 1, 'inGroup' => array(1 => array(1234))), 'twins'); - $this->expectEnabled($c, array('uaid' => 1, 'userID' => 2, 'inGroup' => array(2 => array(2345))), 'twins'); - $this->expectEnabled($c, array('uaid' => 1, 'userID' => 3, 'inGroup' => array(3 => array(3456))), 'other'); - $this->expectDisabled($c, array('uaid' => 0, 'userID' => 4, 'inGroup' => array(4 => array()))); - } - - function testUrlOverride () { - $c = array('enabled' => 0); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => true, 'urlFeatures' => 'foo')); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => true, 'urlFeatures' => 'foo:on')); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => true, 'urlFeatures' => 'foo:bar'), 'bar'); - $this->expectDisabled($c, array('uaid' => '1', 'isInternal' => false, 'urlFeatures' => 'foo')); - $this->expectDisabled($c, array('uaid' => '1', 'isInternal' => false, 'urlFeatures' => 'foo:on')); - $this->expectDisabled($c, array('uaid' => '1', 'isInternal' => false, 'urlFeatures' => 'foo:bar')); - } - - function testPublicUrlOverride () { - $c = array('enabled' => 0, 'public_url_override' => true); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => true, 'urlFeatures' => 'foo')); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => true, 'urlFeatures' => 'foo:on')); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => true, 'urlFeatures' => 'foo:bar'), 'bar'); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => false, 'urlFeatures' => 'foo')); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => false, 'urlFeatures' => 'foo:on')); - $this->expectEnabled($c, array('uaid' => '1', 'isInternal' => false, 'urlFeatures' => 'foo:bar'), 'bar'); - } - - function testBucketBy () { - $c = array('enabled' => 2, 'bucketing' => 'user'); - $this->expectEnabled($c, array('uaid' => 1, 'userID' => .01)); - $this->expectDisabled($c, array('uaid' => 0, 'userID' => .03)); - } - - function testUAIDFallback () { - $c = array('enabled' => 2, 'bucketing' => 'user'); - $this->expectEnabled($c, array('userID' => null, 'uaid' => .01)); - $this->expectDisabled($c, array('userID' => null, 'uaid' => .03)); - } - - /* - * Ignore userID and uuaid in favor of random numbers for bucketing. - */ - function testRandom () { - $c = array('enabled' => 3, 'bucketing' => 'random'); - $this->expectEnabled($c, array('uaid' => 1, 'random' => .00)); - $this->expectEnabled($c, array('uaid' => 1, 'random' => .01)); - $this->expectEnabled($c, array('uaid' => 1, 'random' => .02)); - $this->expectEnabled($c, array('uaid' => 1, 'random' => .02999)); - $this->expectDisabled($c, array('uaid' => 0, 'random' => .03)); - $this->expectDisabled($c, array('uaid' => 0, 'random' => .04)); - $this->expectDisabled($c, array('uaid' => 0, 'random' => .99999)); - } - - /* - * Somewhat indirect test that we cache the value by id: even if - * the config is set up to use a random bucket (i.e. indpendent of - * the id) it should still return the same value for the same id - * which we test by having the two 'random' values returned by the - * test world be ones that would change the enabled status if they - * were both used. - */ - function testRandomCached () { - // Initially enabled - $c = array('enabled' => 3, 'bucketing' => 'random'); - $w = new Testing_Feature_MockWorld(array('uaid' => 1, 'random' => 0)); - $config = new Feature_Config('foo', $c, $w); - $this->assertTrue($config->isEnabled()); - $w->nextRandomValue(.5); - $this->assertTrue($config->isEnabled()); - - // Initially disabled - $c = array('enabled' => 3, 'bucketing' => 'random'); - $w = new Testing_Feature_MockWorld(array('uaid' => 1, 'random' => .5)); - $config = new Feature_Config('foo', $c, $w); - $this->assertFalse($config->isEnabled()); - $w->nextRandomValue(0); - $this->assertFalse($config->isEnabled()); - } - - function testDescription () { - // Default description. - $c = array('enabled' => 'on'); - $w = new Testing_Feature_MockWorld(array()); - $config = new Feature_Config('foo', $c, $w); - $this->assertNotNull($config->description()); - - // Provided description. - $c = array('enabled' => 'on', 'description' => 'The description.'); - $w = new Testing_Feature_MockWorld(array()); - $config = new Feature_Config('foo', $c, $w); - $this->assertEquals($config->description(), 'The description.'); - } - - function testIsEnabledForAcceptsREST_User() { - //we don't want to test the implementation of user bucketing here, just the public API - $user_id = 1; - $user = $this->getMock('REST_User'); - $user->expects($this->once()) - ->method('getUserId') - ->will($this->returnValue($user_id)); - $config = new Feature_Config('foo', array('enabled' => 'off'), new Testing_Feature_MockWorld(array())); - $this->assertFalse($config->isEnabledFor($user)); - } - - function testIsEnabledForAcceptsEtsyModel_User() { - //we don't want to test the implementation of user bucketing here, just the public API - $user = new EtsyModel_User(); - $user->user_id = 1; - $config = new Feature_Config('foo', array('enabled' => 'off'), new Testing_Feature_MockWorld(array())); - $this->assertFalse($config->isEnabledFor($user)); - } - - - //////////////////////////////////////////////////////////////////////// - // Test helper methods. - - /* - * Given a config stanza and a world configuration, we expect that - * isEnabled() will return true and that variant will be a given - * value (default 'on'). - */ - private function expectEnabled ($stanza, $world, $variant = 'on') { - $config = new Feature_Config('foo', $stanza, new Testing_Feature_MockWorld($world)); - $this->assertTrue($config->isEnabled()); - $this->assertEquals($config->variant(), $variant); - - if (is_array($stanza) && array_key_exists('enabled', $stanza) && $stanza['enabled'] === 0) { - unset($stanza['enabled']); - $this->expectEnabled($stanza, $world, $variant); - } - } - - /* - * Given a config stanza and a world configuration, we expect that - * isEnabled() will return false. - */ - private function expectDisabled ($stanza, $world) { - $config = new Feature_Config('foo', $stanza, new Testing_Feature_MockWorld($world)); - $this->assertFalse($config->isEnabled()); - if (is_array($stanza) && array_key_exists('enabled', $stanza) && $stanza['enabled'] === 0) { - unset($stanza['enabled']); - $this->expectDisabled($stanza, $world); - } - } -} diff --git a/phpunit/Feature/LoggerTest.php b/phpunit/Feature/LoggerTest.php deleted file mode 100644 index cd1c3d9..0000000 --- a/phpunit/Feature/LoggerTest.php +++ /dev/null @@ -1,42 +0,0 @@ -assertEquals('', Feature_Logger::getGAJavascript(array())); - } - - function testLogOne() { - $selections = array(); - $selections[] = array('TEST_key1', 'TEST_var1', 123); - $js = Feature_Logger::getGAJavascript($selections); - $this->assertEquals("Etsy.GA.track(['_setCustomVar', 2, 'AB', 'TEST_key1.TEST_var1', 3]);", $js); - } - - function testLogTwo() { - $selections = array(); - $selections[] = array('TEST_key1', 'TEST_var1', 123); - $selections[] = array('foo', 'bar', 123); - $js = Feature_Logger::getGAJavascript($selections); - $this->assertEquals("Etsy.GA.track(['_setCustomVar', 2, 'AB', 'TEST_key1.TEST_var1..foo.bar', 3]);", $js); - } - - function testTooLong() { - $selections = array(); - $pairs = array(); - foreach (array('a', 'b', 'c', 'd', 'e') as $x) { - $selections[] = array($x, 'xxxxxxxxxx', 123); - $pairs[] = "$x.xxxxxxxxxx"; - } - // This one should not be included in the Javascript because - // we already have 12*5=60 chars and this pair would add three - // more pushing us over the limit of 62. - $selections[] = array('f', 'x', 123); - $value = implode('..', $pairs); - $js = Feature_Logger::getGAJavascript($selections); - $this->assertEquals("Etsy.GA.track(['_setCustomVar', 2, 'AB', '$value', 3]);", $js); - } -} diff --git a/phpunit/Feature/WorldTest.php b/phpunit/Feature/WorldTest.php deleted file mode 100644 index 423f162..0000000 --- a/phpunit/Feature/WorldTest.php +++ /dev/null @@ -1,153 +0,0 @@ -uaid = UAIDCookie::getSecureCookie(); - $this->assertNotNull($this->uaid); - - $logger = $this->getMock('Logger', array('log')); - $this->world = new Feature_World($logger); - $this->user_id = 991; - - $this->setLoggedUserId(null); - $this->assertNull(Std::loggedUser()); - } - - function testIsAdminWithBlankUAIDCookie() { - $this->setLoggedUserId($this->user_id); - - $this->assertFalse($this->world->isAdmin($this->user_id)); - } - - function testIsAdminWithValidNonAdminUserUAIDCookie() { - $this->setLoggedUserId($this->user_id); - $this->uaid->set(UAIDCookie::USER_ID_ATTRIBUTE, $this->user_id); - - $this->assertFalse($this->world->isAdmin($this->user_id)); - } - - function testIsAdminWithValidAdminUAIDCookie() { - $this->setLoggedUserId($this->user_id); - $this->uaid->set(UAIDCookie::USER_ID_ATTRIBUTE, $this->user_id); - $this->uaid->set(UAIDCookie::ADMIN_ATTRIBUTE, '1'); - - $this->assertTrue($this->world->isAdmin($this->user_id)); - } - - function testIsAdminWithNonLoggedInAdminAndValidAdminUAIDCookie() { - $this->setLoggedUserId(null); - $this->uaid->set(UAIDCookie::USER_ID_ATTRIBUTE, $this->user_id); - $this->uaid->set(UAIDCookie::ADMIN_ATTRIBUTE, '1'); - - $this->assertFalse($this->world->isAdmin($this->user_id)); - } - - function testIsAdminWithLoggedInAdminUserAndBlankUAIDCookie() { - $user = $this->adminUser(); - $this->setLoggedUserId($user->user_id); - - $this->assertTrue($this->world->isAdmin($user->user_id)); - } - - function testIsAdminWithLoggedInNonAdminUserAndBlankUAIDCookie() { - $user = $this->nonAdminUser(); - $this->setLoggedUserId($user->user_id); - - $this->assertFalse($this->world->isAdmin($user->user_id)); - } - - function testIsAdminWithNonLoggedInAdminUserAndBlankUAIDCookie() { - $user = $this->adminUser(); - $this->setLoggedUserId(null); - - $this->assertTrue($this->world->isAdmin($user->user_id)); - } - - function testIsAdminWithNonLoggedInNonAdminUserAndBlankUAIDCookie() { - $user = $this->nonAdminUser(); - $this->setLoggedUserId(null); - - $this->assertFalse($this->world->isAdmin($user->user_id)); - } - - function testAtlasWorld() { - $user = $this->atlasUser(); - $this->setLoggedUserId($user->id); - $this->setAtlasRequest(true); - - $this->assertFalse($this->world->isAdmin($user->id)); - $this->assertFalse($this->world->inGroup($user->id, 1)); - $this->assertEquals($user->id, $this->world->userID()); - - $this->setAtlasRequest(false); - } - - function testHash() { - $this->assertInternalType('float', $this->world->hash('somevalue')); - - $this->assertEquals( - $this->world->hash('somevalue'), - $this->world->hash('somevalue'), - 'ensure return value is consistent' - ); - - $this->assertGreaterThanOrEqual(0, $this->world->hash('somevalue')); - $this->assertLessThan(1, $this->world->hash('somevalue')); - } - - protected function getDatabaseConfigs() { - $index_yml = dirname(__FILE__) . '/data/world/etsy_index.yml'; - if (!file_exists($index_yml)) { - throw new Exception($index_yml . ' does not exist'); - } - $builder = new PHPUnit_Extensions_MultipleDatabase_DatabaseConfig_Builder(); - $etsy_index = $builder - ->connection(Testing_EtsyORM_Connections::ETSY_INDEX()) - ->dataSet(new PHPUnit_Extensions_Database_DataSet_YamlDataSet($index_yml)) - ->build(); - - $aux_yml = dirname(__FILE__) . '/data/world/etsy_aux.yml'; - if (!file_exists($aux_yml)) { - throw new Exception($aux_yml . ' does not exist'); - } - $builder = new PHPUnit_Extensions_MultipleDatabase_DatabaseConfig_Builder(); - $etsy_aux = $builder - ->connection(Testing_EtsyORM_Connections::ETSY_AUX()) - ->dataSet(new PHPUnit_Extensions_Database_DataSet_YamlDataSet($aux_yml)) - ->build(); - - return array($etsy_index, $etsy_aux); - } - - private function nonAdminUser() { - return EtsyORM::getFinder('User')->find(1); - } - - private function adminUser() { - return EtsyORM::getFinder('User')->find(2); - } - - private function atlasUser() { - return EtsyORM::getFinder('Staff')->find(3); - } - - private function setAtlasRequest($is_atlas) { - $_SERVER["atlas_request"] = $is_atlas ? 1 : 0; - } - - private function setLoggedUserId($user_id) { - //Std::loggedUser() uses this global - $GLOBALS['cookie_user_id'] = $user_id; - } -} diff --git a/phpunit/Feature/data/world/etsy_aux.yml b/phpunit/Feature/data/world/etsy_aux.yml deleted file mode 100644 index fc27946..0000000 --- a/phpunit/Feature/data/world/etsy_aux.yml +++ /dev/null @@ -1,6 +0,0 @@ -staff: - - - id: 3 - auth_username: 'staff_member' - create_date: 0 - update_date: 0 diff --git a/phpunit/Feature/data/world/etsy_index.yml b/phpunit/Feature/data/world/etsy_index.yml deleted file mode 100644 index d9ddaac..0000000 --- a/phpunit/Feature/data/world/etsy_index.yml +++ /dev/null @@ -1,14 +0,0 @@ -users_index: - - - user_id: 1 - user_shard: 1 - login_name: peter - primary_email: foo@etsycorp.com - is_admin: 0 - - - user_id: 2 - user_shard: 1 - login_name: paul - primary_email: bar@etsycorp.com - is_admin: 1 - diff --git a/src/Bucketing/Enum.php b/src/Bucketing/Enum.php new file mode 100644 index 0000000..cca8d8d --- /dev/null +++ b/src/Bucketing/Enum.php @@ -0,0 +1,19 @@ + new Random(), + static::ID => new Id() + }; + } +} diff --git a/src/Bucketing/Id.php b/src/Bucketing/Id.php new file mode 100644 index 0000000..a97f779 --- /dev/null +++ b/src/Bucketing/Id.php @@ -0,0 +1,29 @@ +randomizer = new Randomizer(new Xoshiro256StarStar()); + } + + public function strToIntHash(string $idToHash): int + { + return $this->randomizer->getInt(0, 99); + } +} diff --git a/src/Bucketing/Type.php b/src/Bucketing/Type.php new file mode 100644 index 0000000..53c7ece --- /dev/null +++ b/src/Bucketing/Type.php @@ -0,0 +1,14 @@ +configurations = array_map( + $this->buildConfig(...), + $configurations + ); + } + + public function get(string $featureName): Config + { + return $this->configurations[$featureName]; + } + + private function buildConfig(array $config): Config + { + return new Config($config); + } +} diff --git a/src/Configurations/Config.php b/src/Configurations/Config.php new file mode 100644 index 0000000..1c8ad15 --- /dev/null +++ b/src/Configurations/Config.php @@ -0,0 +1,66 @@ +variantIntegerRanges = $this->calculateIntegerRangeFromVariants( + $config['variants'] ?? [] + ); + + $bucketingOption = BucketOptions::tryFrom($config['bucketing'] ?? ''); + $bucketingOption ??= BucketOptions::RANDOM; + $this->bucketing = $bucketingOption->getBucketingClass(); + } + + /** + * Using a random 0 - 99 number or a 0 - 99 number hashed from an id, + * Select the variant where this random or hashed integer falls within it's + * calculated integer range. + */ + public function getEnabledVariant(string $id): string + { + $hashOrRandomNumber = $this->bucketing->strToIntHash($id); + + foreach ($this->variantIntegerRanges as $variant => $variantRange) { + if ($hashOrRandomNumber < $variantRange) { + return $variant; + } + } + + return ''; + } + + /** + * Parse the 'variants' property of the feature's config stanza. + * Returns the upper-boundary of the variants percentage and uses that + * upper-boundary integer as its range of integers. + */ + private function calculateIntegerRangeFromVariants(array $variants): array + { + $total = 0; + $percentages = []; + + foreach ($variants as $variant => $percent) { + $total += $percent; + $percentages[$variant] = $total; + } + + asort($percentages, SORT_NUMERIC); + + return $percentages; + } +} diff --git a/src/Features.php b/src/Features.php new file mode 100644 index 0000000..1f46798 --- /dev/null +++ b/src/Features.php @@ -0,0 +1,57 @@ +isEnabled(featureName: 'foo'); + * Feature->variant(featureName: 'foo'); + * + * For cases when we want to bucket on a user other than the currently logged in + * user, on something else entirely (such as a shop ID), or any arbitrary + * string, pass the string value as a second parameter. + * + * Feature->isEnabled(featureName: 'foo', id: $id); + * Feature->getEnabledVariant(featureName: 'foo', id: $id); + */ +final readonly class Features +{ + private Collection $features; + + public function __construct(array $features) + { + $this->features = new Collection($features); + } + + /** + * Test whether the named feature is enabled for a given user + * or arbitrary string. + */ + public function isEnabled(string $featureName, string $id = ''): bool + { + return (bool) $this->getEnabledVariant( + featureName: $featureName, + id: $id + ); + } + + /** + * Get the name of the enabled variant for the named feature for the given + * id. Returns an empty string if the feature is not enabled. + */ + public function getEnabledVariant( + string $featureName, + string $id = '' + ): string + { + return $this->features->get($featureName)->getEnabledVariant($id); + } +} diff --git a/tests/FeatureTest.php b/tests/FeatureTest.php new file mode 100644 index 0000000..75fbe90 --- /dev/null +++ b/tests/FeatureTest.php @@ -0,0 +1,160 @@ + [ + 'variants' => ['enabled' => 100] + ], + 'test_feature_2' => [ + 'variants' => ['enabled' => 0] + ], + 'test_feature_3' => [ + 'variants' => ['enabled' => 50], + 'bucketing' => 'id' + ], + 'test_feature_4' => [ + 'variants' => [ + 'test1' => 20, + 'test2' => 30, + 'test3' => 15, + 'test4' => 35 + ], + 'bucketing' => 'id' + ], + 'test_feature_5' => [ + 'variants' => ['enabled' => 0], + 'bucketing' => 'random' + ], + 'test_feature_6' => [ + 'variants' => ['enabled' => 0], + 'bucketing' => 'id' + ], + 'test_feature_7' => [ + 'variants' => ['enabled' => 100], + 'bucketing' => 'random' + ], + 'test_feature_8' => [ + 'variants' => ['enabled' => 100], + 'bucketing' => 'id' + ] + ]); + + $this->assertEquals($feature->isEnabled('test_feature_1'), true); + $this->assertEquals($feature->isEnabled('test_feature_2'), false); + $this->assertEquals($feature->isEnabled('test_feature_3'), true); + $this->assertEquals($feature->isEnabled('test_feature_4'), true); + $this->assertEquals($feature->isEnabled('test_feature_5'), false); + $this->assertEquals($feature->isEnabled('test_feature_6'), false); + $this->assertEquals($feature->isEnabled('test_feature_7'), true); + $this->assertEquals($feature->isEnabled('test_feature_8'), true); + + $this->assertEquals( + $feature->isEnabled('test_feature_1', 'test'), + true + ); + $this->assertEquals( + $feature->isEnabled('test_feature_2', 'test'), + false + ); + $this->assertEquals( + $feature->isEnabled('test_feature_3', 'test'), + false + ); + $this->assertEquals( + $feature->isEnabled('test_feature_4', 'test'), + true + ); + $this->assertEquals( + $feature->isEnabled('test_feature_5', 'test'), + false + ); + $this->assertEquals( + $feature->isEnabled('test_feature_6', 'test'), + false + ); + $this->assertEquals( + $feature->isEnabled('test_feature_7', 'test'), + true + ); + $this->assertEquals( + $feature->isEnabled('test_feature_8', 'test'), + true + ); + + $this->assertEquals( + $feature->getEnabledVariant('test_feature_1'), + 'enabled' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_2'), + '' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_3'), + 'enabled' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_4'), + 'test1' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_5'), + '' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_6'), + '' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_7'), + 'enabled' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_8'), + 'enabled' + ); + + $this->assertEquals( + $feature->getEnabledVariant('test_feature_1', 'test'), + 'enabled' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_2', 'test'), + '' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_3', 'test'), + '' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_4', 'test'), + 'test3' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_5', 'test'), + '' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_6', 'test'), + '' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_7', 'test'), + 'enabled' + ); + $this->assertEquals( + $feature->getEnabledVariant('test_feature_8', 'test'), + 'enabled' + ); + } +}