diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f071d9b..0589a487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to the LaunchDarkly Python SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [6.4.0] - 2018-08-29 +### Added: +- The new `LDClient` method `variation_detail` allows you to evaluate a feature flag (using the same parameters as you would for `variation`) and receive more information about how the value was calculated. This information is returned in an `EvaluationDetail` object, which contains both the result value and a "reason" object which will tell you, for instance, if the user was individually targeted for the flag or was matched by one of the flag's rules, or if the flag returned the default value due to an error. + +### Fixed: +- When evaluating a prerequisite feature flag, the analytics event for the evaluation did not include the result value if the prerequisite flag was off. + ## [6.3.0] - 2018-08-27 ### Added: - The new `LDClient` method `all_flags_state()` should be used instead of `all_flags()` if you are passing flag data to the front end for use with the JavaScript SDK. It preserves some flag metadata that the front end requires in order to send analytics events correctly. Versions 2.5.0 and above of the JavaScript SDK are able to use this metadata, but the output of `all_flags_state()` will still work with older versions. diff --git a/ldclient/client.py b/ldclient/client.py index d0256644..d635e09c 100644 --- a/ldclient/client.py +++ b/ldclient/client.py @@ -3,13 +3,14 @@ import hashlib import hmac import threading +import traceback from builtins import object from ldclient.config import Config as Config from ldclient.event_processor import NullEventProcessor from ldclient.feature_requester import FeatureRequesterImpl -from ldclient.flag import evaluate +from ldclient.flag import EvaluationDetail, evaluate, error_reason from ldclient.flags_state import FeatureFlagsState from ldclient.polling import PollingUpdateProcessor from ldclient.streaming import StreamingUpdateProcessor @@ -184,17 +185,50 @@ def variation(self, key, user, default): available from LaunchDarkly :return: one of the flag's variation values, or the default value """ + return self._evaluate_internal(key, user, default, False).value + + def variation_detail(self, key, user, default): + """Determines the variation of a feature flag for a user, like `variation`, but also + provides additional information about how this value was calculated. + + The return value is an EvaluationDetail object, which has three properties: + + `value`: the value that was calculated for this user (same as the return value + of `variation`) + + `variation_index`: the positional index of this value in the flag, e.g. 0 for the + first variation - or `None` if the default value was returned + + `reason`: a hash describing the main reason why this value was selected. + + The `reason` will also be included in analytics events, if you are capturing + detailed event data for this flag. + + :param string key: the unique key for the feature flag + :param dict user: a dictionary containing parameters for the end user requesting the flag + :param object default: the default value of the flag, to be used if the value is not + available from LaunchDarkly + :return: an EvaluationDetail object describing the result + :rtype: EvaluationDetail + """ + return self._evaluate_internal(key, user, default, True) + + def _evaluate_internal(self, key, user, default, include_reasons_in_events): default = self._config.get_default(key, default) - if user is not None: - self._sanitize_user(user) if self._config.offline: - return default + return EvaluationDetail(default, None, error_reason('CLIENT_NOT_READY')) + + if user is not None: + self._sanitize_user(user) - def send_event(value, version=None): - self._send_event({'kind': 'feature', 'key': key, 'user': user, 'variation': None, - 'value': value, 'default': default, 'version': version, - 'trackEvents': False, 'debugEventsUntilDate': None}) + def send_event(value, variation=None, flag=None, reason=None): + self._send_event({'kind': 'feature', 'key': key, 'user': user, + 'value': value, 'variation': variation, 'default': default, + 'version': flag.get('version') if flag else None, + 'trackEvents': flag.get('trackEvents') if flag else None, + 'debugEventsUntilDate': flag.get('debugEventsUntilDate') if flag else None, + 'reason': reason if include_reasons_in_events else None}) if not self.is_initialized(): if self._store.initialized: @@ -202,51 +236,40 @@ def send_event(value, version=None): else: log.warn("Feature Flag evaluation attempted before client has initialized! Feature store unavailable - returning default: " + str(default) + " for feature key: " + key) - send_event(default) - return default - + reason = error_reason('CLIENT_NOT_READY') + send_event(default, None, None, reason) + return EvaluationDetail(default, None, reason) + if user is not None and user.get('key', "") == "": log.warn("User key is blank. Flag evaluation will proceed, but the user will not be stored in LaunchDarkly.") - def cb(flag): - try: - if not flag: - log.info("Feature Flag key: " + key + " not found in Feature Store. Returning default.") - send_event(default) - return default - - return self._evaluate_and_send_events(flag, user, default) - - except Exception as e: - log.error("Exception caught in variation: " + e.message + " for flag key: " + key + " and user: " + str(user)) - send_event(default) - - return default - - return self._store.get(FEATURES, key, cb) - - def _evaluate(self, flag, user): - return evaluate(flag, user, self._store) - - def _evaluate_and_send_events(self, flag, user, default): - if user is None or user.get('key') is None: - log.warn("Missing user or user key when evaluating Feature Flag key: " + flag.get('key') + ". Returning default.") - value = default - variation = None + flag = self._store.get(FEATURES, key, lambda x: x) + if not flag: + reason = error_reason('FLAG_NOT_FOUND') + send_event(default, None, None, reason) + return EvaluationDetail(default, None, reason) else: - result = evaluate(flag, user, self._store) - for event in result.events or []: - self._send_event(event) - value = default if result.value is None else result.value - variation = result.variation - - self._send_event({'kind': 'feature', 'key': flag.get('key'), - 'user': user, 'variation': variation, 'value': value, - 'default': default, 'version': flag.get('version'), - 'trackEvents': flag.get('trackEvents'), - 'debugEventsUntilDate': flag.get('debugEventsUntilDate')}) - return value + if user is None or user.get('key') is None: + reason = error_reason('USER_NOT_SPECIFIED') + send_event(default, None, flag, reason) + return EvaluationDetail(default, None, reason) + try: + result = evaluate(flag, user, self._store, include_reasons_in_events) + for event in result.events or []: + self._send_event(event) + detail = result.detail + if detail.is_default_value(): + detail = EvaluationDetail(default, None, detail.reason) + send_event(detail.value, detail.variation_index, flag, detail.reason) + return detail + except Exception as e: + log.error("Unexpected error while evaluating feature flag \"%s\": %s" % (key, e)) + log.debug(traceback.format_exc()) + reason = error_reason('EXCEPTION') + send_event(default, None, flag, reason) + return EvaluationDetail(default, None, reason) + def all_flags(self, user): """Returns all feature flag values for the given user. @@ -272,7 +295,8 @@ def all_flags_state(self, user, **kwargs): :param dict user: the end user requesting the feature flags :param kwargs: optional parameters affecting how the state is computed: set `client_side_only=True` to limit it to only flags that are marked for use with the - client-side SDK (by default, all flags are included) + client-side SDK (by default, all flags are included); set `with_reasons=True` to + include evaluation reasons in the state (see `variation_detail`) :return: a FeatureFlagsState object (will never be None; its 'valid' property will be False if the client is offline, has not been initialized, or the user is None or has no key) :rtype: FeatureFlagsState @@ -294,6 +318,7 @@ def all_flags_state(self, user, **kwargs): state = FeatureFlagsState(True) client_only = kwargs.get('client_side_only', False) + with_reasons = kwargs.get('with_reasons', False) try: flags_map = self._store.all(FEATURES, lambda x: x) except Exception as e: @@ -304,11 +329,14 @@ def all_flags_state(self, user, **kwargs): if client_only and not flag.get('clientSide', False): continue try: - result = self._evaluate(flag, user) - state.add_flag(flag, result.value, result.variation) + detail = evaluate(flag, user, self._store, False).detail + state.add_flag(flag, detail.value, detail.variation_index, + detail.reason if with_reasons else None) except Exception as e: log.error("Error evaluating flag \"%s\" in all_flags_state: %s" % (key, e)) - state.add_flag(flag, None, None) + log.debug(traceback.format_exc()) + reason = {'kind': 'ERROR', 'errorKind': 'EXCEPTION'} + state.add_flag(flag, None, None, reason if with_reasons else None) return state diff --git a/ldclient/event_processor.py b/ldclient/event_processor.py index fa6061b4..3b89420f 100644 --- a/ldclient/event_processor.py +++ b/ldclient/event_processor.py @@ -84,6 +84,8 @@ def make_output_event(self, e): out['user'] = self._user_filter.filter_user_props(e['user']) else: out['userKey'] = e['user'].get('key') + if e.get('reason'): + out['reason'] = e.get('reason') return out elif kind == 'identify': return { diff --git a/ldclient/flag.py b/ldclient/flag.py index 7b0e9ed3..d4fcbdf3 100644 --- a/ldclient/flag.py +++ b/ldclient/flag.py @@ -16,81 +16,155 @@ log = logging.getLogger(sys.modules[__name__].__name__) -EvalResult = namedtuple('EvalResult', ['variation', 'value', 'events']) +class EvaluationDetail(object): + """ + The return type of LDClient.variation_detail, combining the result of a flag evaluation + with information about how it was calculated. + """ + def __init__(self, value, variation_index, reason): + self.__value = value + self.__variation_index = variation_index + self.__reason = reason + + @property + def value(self): + """The result of the flag evaluation. This will be either one of the flag's + variations or the default value that was passed to the variation() method. + """ + return self.__value + + @property + def variation_index(self): + """The index of the returned value within the flag's list of variations, e.g. + 0 for the first variation - or None if the default value was returned. + """ + return self.__variation_index + + @property + def reason(self): + """A dictionary describing the main factor that influenced the flag evaluation value. + It contains the following properties: + + 'kind': The general category of reason, as follows: 'OFF' - the flag was off; + 'FALLTHROUGH' - the flag was on but the user did not match any targets or rules; + 'TARGET_MATCH' - the user was specifically targeted for this flag; 'RULE_MATCH' - + the user matched one of the flag's rules; 'PREREQUISITE_FAILED' - the flag was + considered off because it had at least one prerequisite flag that did not return + the desired variation; 'ERROR' - the flag could not be evaluated due to an + unexpected error. + + 'ruleIndex', 'ruleId': The positional index and unique identifier of the matched + rule, if the kind was 'RULE_MATCH' + + 'prerequisiteKey': The flag key of the prerequisite that failed, if the kind was + 'PREREQUISITE_FAILED' + + 'errorKind': further describes the nature of the error if the kind was 'ERROR', + e.g. 'FLAG_NOT_FOUND' + """ + return self.__reason + + def is_default_value(self): + """Returns True if the flag evaluated to the default value rather than one of its + variations. + """ + return self.__variation_index is None + + def __eq__(self, other): + return self.value == other.value and self.variation_index == other.variation_index and self.reason == other.reason + + def __ne__(self, other): + return not self.__eq__(other) + + def __str__(self): + return "(value=%s, variation_index=%s, reason=%s)" % (self.value, self.variation_index, self.reason) + + def __repr__(self): + return self.__str__() + + +EvalResult = namedtuple('EvalResult', ['detail', 'events']) + + +def error_reason(error_kind): + return {'kind': 'ERROR', 'errorKind': error_kind} + + +def evaluate(flag, user, store, include_reasons_in_events = False): + prereq_events = [] + detail = _evaluate(flag, user, store, prereq_events, include_reasons_in_events) + return EvalResult(detail = detail, events = prereq_events) +def _evaluate(flag, user, store, prereq_events, include_reasons_in_events): + if not flag.get('on', False): + return _get_off_value(flag, {'kind': 'OFF'}) + + prereq_failure_reason = _check_prerequisites(flag, user, store, prereq_events, include_reasons_in_events) + if prereq_failure_reason is not None: + return _get_off_value(flag, prereq_failure_reason) -def evaluate(flag, user, store): - prereq_events = [] - if flag.get('on', False): - variation, value, prereq_events = _evaluate(flag, user, store) - if value is not None: - return EvalResult(variation = variation, value = value, events = prereq_events) + # Check to see if any user targets match: + for target in flag.get('targets') or []: + for value in target.get('values') or []: + if value == user['key']: + return _get_variation(flag, target.get('variation'), {'kind': 'TARGET_MATCH'}) - off_var = flag.get('offVariation') - off_value = None if off_var is None else _get_variation(flag, off_var) - return EvalResult(variation = off_var, value = off_value, events = prereq_events) + # Now walk through the rules to see if any match + for index, rule in enumerate(flag.get('rules') or []): + if _rule_matches_user(rule, user, store): + return _get_value_for_variation_or_rollout(flag, rule, user, + {'kind': 'RULE_MATCH', 'ruleIndex': index, 'ruleId': rule.get('id')}) + + # Walk through fallthrough and see if it matches + if flag.get('fallthrough') is not None: + return _get_value_for_variation_or_rollout(flag, flag['fallthrough'], user, {'kind': 'FALLTHROUGH'}) -def _evaluate(flag, user, store, prereq_events=None): - events = prereq_events or [] +def _check_prerequisites(flag, user, store, events, include_reasons_in_events): failed_prereq = None - prereq_var = None - prereq_value = None + prereq_res = None for prereq in flag.get('prerequisites') or []: prereq_flag = store.get(FEATURES, prereq.get('key'), lambda x: x) if prereq_flag is None: log.warn("Missing prereq flag: " + prereq.get('key')) failed_prereq = prereq - break - if prereq_flag.get('on', False) is True: - prereq_var, prereq_value, events = _evaluate(prereq_flag, user, store, events) - if prereq_var is None or not prereq_var == prereq.get('variation'): - failed_prereq = prereq else: - failed_prereq = prereq - - event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, 'variation': prereq_var, - 'value': prereq_value, 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key'), - 'trackEvents': prereq_flag.get('trackEvents'), - 'debugEventsUntilDate': prereq_flag.get('debugEventsUntilDate')} - events.append(event) - - if failed_prereq is not None: - return None, None, events - - index = _evaluate_index(flag, user, store) - return index, _get_variation(flag, index), events - - -def _evaluate_index(feature, user, store): - # Check to see if any user targets match: - for target in feature.get('targets') or []: - for value in target.get('values') or []: - if value == user['key']: - return target.get('variation') - - # Now walk through the rules to see if any match - for rule in feature.get('rules') or []: - if _rule_matches_user(rule, user, store): - return _variation_index_for_user(feature, rule, user) + prereq_res = _evaluate(prereq_flag, user, store, events, include_reasons_in_events) + # Note that if the prerequisite flag is off, we don't consider it a match no matter what its + # off variation was. But we still need to evaluate it in order to generate an event. + if (not prereq_flag.get('on', False)) or prereq_res.variation_index != prereq.get('variation'): + failed_prereq = prereq + event = {'kind': 'feature', 'key': prereq.get('key'), 'user': user, + 'variation': prereq_res.variation_index, 'value': prereq_res.value, + 'version': prereq_flag.get('version'), 'prereqOf': flag.get('key'), + 'trackEvents': prereq_flag.get('trackEvents'), + 'debugEventsUntilDate': prereq_flag.get('debugEventsUntilDate'), + 'reason': prereq_res.reason if prereq_res and include_reasons_in_events else None} + events.append(event) + if failed_prereq: + return {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': failed_prereq.get('key')} + return None - # Walk through fallthrough and see if it matches - if feature.get('fallthrough') is not None: - return _variation_index_for_user(feature, feature['fallthrough'], user) - return None +def _get_variation(flag, variation, reason): + vars = flag.get('variations') or [] + if variation < 0 or variation >= len(vars): + return EvaluationDetail(None, None, error_reason('MALFORMED_FLAG')) + return EvaluationDetail(vars[variation], variation, reason) -def _get_variation(feature, index): - if index is not None and index < len(feature['variations']): - return feature['variations'][index] - return None +def _get_off_value(flag, reason): + off_var = flag.get('offVariation') + if off_var is None: + return EvaluationDetail(None, None, reason) + return _get_variation(flag, off_var, reason) -def _get_off_variation(feature): - if feature.get('offVariation') is not None: - return _get_variation(feature, feature.get('offVariation')) - return None +def _get_value_for_variation_or_rollout(flag, vr, user, reason): + index = _variation_index_for_user(flag, vr, user) + if index is None: + return EvaluationDetail(None, None, error_reason('MALFORMED_FLAG')) + return _get_variation(flag, index, reason) def _get_user_attribute(user, attr): diff --git a/ldclient/flags_state.py b/ldclient/flags_state.py index 7e8ab3b9..c76b4908 100644 --- a/ldclient/flags_state.py +++ b/ldclient/flags_state.py @@ -12,13 +12,15 @@ def __init__(self, valid): self.__flag_metadata = {} self.__valid = valid - def add_flag(self, flag, value, variation): + def add_flag(self, flag, value, variation, reason): """Used internally to build the state map.""" key = flag['key'] self.__flag_values[key] = value meta = { 'version': flag.get('version'), 'trackEvents': flag.get('trackEvents') } if variation is not None: meta['variation'] = variation + if reason is not None: + meta['reason'] = reason if flag.get('debugEventsUntilDate') is not None: meta['debugEventsUntilDate'] = flag.get('debugEventsUntilDate') self.__flag_metadata[key] = meta @@ -37,6 +39,15 @@ def get_flag_value(self, key): """ return self.__flag_values.get(key) + def get_flag_reason(self, key): + """Returns the evaluation reason for an individual feature flag at the time the state was recorded. + :param string key: the feature flag key + :return: a dictionary describing the reason; None if reasons were not recorded, or if there was no + such flag + """ + meta = self.__flag_metadata.get(key) + return None if meta is None else meta.get('reason') + def to_values_map(self): """Returns a dictionary of flag keys to flag values. If the flag would have evaluated to the default value, its value will be None. diff --git a/ldclient/version.py b/ldclient/version.py index 45311203..1d01958d 100644 --- a/ldclient/version.py +++ b/ldclient/version.py @@ -1 +1 @@ -VERSION = "6.2.0" +VERSION = "6.4.0" diff --git a/setup.py b/setup.py index e471ab74..95c419f3 100644 --- a/setup.py +++ b/setup.py @@ -13,7 +13,7 @@ def parse_requirements(filename): return [line for line in lineiter if line and not line.startswith("#")] -ldclient_version='6.3.0' +ldclient_version='6.4.0' # parse_requirements() returns generator of pip.req.InstallRequirement objects install_reqs = parse_requirements('requirements.txt') diff --git a/testing/test_flag.py b/testing/test_flag.py index 29d2bb61..97f64af0 100644 --- a/testing/test_flag.py +++ b/testing/test_flag.py @@ -1,32 +1,65 @@ import pytest from ldclient.feature_store import InMemoryFeatureStore -from ldclient.flag import EvalResult, _bucket_user, evaluate +from ldclient.flag import EvaluationDetail, EvalResult, _bucket_user, evaluate from ldclient.versioned_data_kind import FEATURES, SEGMENTS empty_store = InMemoryFeatureStore() +def make_boolean_flag_with_rules(rules): + return { + 'key': 'feature', + 'on': True, + 'rules': rules, + 'fallthrough': { 'variation': 0 }, + 'variations': [ False, True ], + 'salt': '' + } + + def test_flag_returns_off_variation_if_flag_is_off(): flag = { 'key': 'feature', 'on': False, 'offVariation': 1, - 'fallthrough': { 'variation': 0 }, 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == EvalResult(1, 'b', []) + detail = EvaluationDetail('b', 1, {'kind': 'OFF'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) def test_flag_returns_none_if_flag_is_off_and_off_variation_is_unspecified(): flag = { 'key': 'feature', 'on': False, - 'fallthrough': { 'variation': 0 }, 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == EvalResult(None, None, []) + detail = EvaluationDetail(None, None, {'kind': 'OFF'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_off_variation_is_too_high(): + flag = { + 'key': 'feature', + 'on': False, + 'offVariation': 999, + 'variations': ['a', 'b', 'c'] + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_off_variation_is_negative(): + flag = { + 'key': 'feature', + 'on': False, + 'offVariation': -1, + 'variations': ['a', 'b', 'c'] + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) def test_flag_returns_off_variation_if_prerequisite_not_found(): flag = { @@ -38,7 +71,36 @@ def test_flag_returns_off_variation_if_prerequisite_not_found(): 'variations': ['a', 'b', 'c'] } user = { 'key': 'x' } - assert evaluate(flag, user, empty_store) == EvalResult(1, 'b', []) + detail = EvaluationDetail('b', 1, {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': 'badfeature'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_off_variation_and_event_if_prerequisite_is_off(): + store = InMemoryFeatureStore() + flag = { + 'key': 'feature0', + 'on': True, + 'prerequisites': [{'key': 'feature1', 'variation': 1}], + 'fallthrough': { 'variation': 0 }, + 'offVariation': 1, + 'variations': ['a', 'b', 'c'], + 'version': 1 + } + flag1 = { + 'key': 'feature1', + 'off': False, + 'offVariation': 1, + # note that even though it returns the desired variation, it is still off and therefore not a match + 'fallthrough': { 'variation': 0 }, + 'variations': ['d', 'e'], + 'version': 2, + 'trackEvents': False + } + store.upsert(FEATURES, flag1) + user = { 'key': 'x' } + detail = EvaluationDetail('b', 1, {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': 'feature1'}) + events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 1, 'value': 'e', + 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None, 'reason': None}] + assert evaluate(flag, user, store) == EvalResult(detail, events_should_be) def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): store = InMemoryFeatureStore() @@ -61,9 +123,10 @@ def test_flag_returns_off_variation_and_event_if_prerequisite_is_not_met(): } store.upsert(FEATURES, flag1) user = { 'key': 'x' } + detail = EvaluationDetail('b', 1, {'kind': 'PREREQUISITE_FAILED', 'prerequisiteKey': 'feature1'}) events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 0, 'value': 'd', - 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None}] - assert evaluate(flag, user, store) == EvalResult(1, 'b', events_should_be) + 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None, 'reason': None}] + assert evaluate(flag, user, store) == EvalResult(detail, events_should_be) def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_rules(): store = InMemoryFeatureStore() @@ -86,44 +149,104 @@ def test_flag_returns_fallthrough_and_event_if_prereq_is_met_and_there_are_no_ru } store.upsert(FEATURES, flag1) user = { 'key': 'x' } + detail = EvaluationDetail('a', 0, {'kind': 'FALLTHROUGH'}) events_should_be = [{'kind': 'feature', 'key': 'feature1', 'variation': 1, 'value': 'e', - 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None}] - assert evaluate(flag, user, store) == EvalResult(0, 'a', events_should_be) + 'version': 2, 'user': user, 'prereqOf': 'feature0', 'trackEvents': False, 'debugEventsUntilDate': None, 'reason': None}] + assert evaluate(flag, user, store) == EvalResult(detail, events_should_be) -def test_flag_matches_user_from_targets(): +def test_flag_returns_error_if_fallthrough_variation_is_too_high(): flag = { - 'key': 'feature0', + 'key': 'feature', 'on': True, - 'targets': [{ 'values': ['whoever', 'userkey'], 'variation': 2 }], - 'fallthrough': { 'variation': 0 }, - 'offVariation': 1, + 'fallthrough': {'variation': 999}, 'variations': ['a', 'b', 'c'] } - user = { 'key': 'userkey' } - assert evaluate(flag, user, empty_store) == EvalResult(2, 'c', []) + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) -def test_flag_matches_user_from_rules(): +def test_flag_returns_error_if_fallthrough_variation_is_negative(): + flag = { + 'key': 'feature', + 'on': True, + 'fallthrough': {'variation': -1}, + 'variations': ['a', 'b', 'c'] + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_fallthrough_has_no_variation_or_rollout(): + flag = { + 'key': 'feature', + 'on': True, + 'fallthrough': {}, + 'variations': ['a', 'b', 'c'] + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_fallthrough_has_rollout_with_no_variations(): + flag = { + 'key': 'feature', + 'on': True, + 'fallthrough': {'rollout': {'variations': []}}, + 'variations': ['a', 'b', 'c'], + 'salt': '' + } + user = { 'key': 'x' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_matches_user_from_targets(): flag = { 'key': 'feature0', 'on': True, - 'rules': [ - { - 'clauses': [ - { - 'attribute': 'key', - 'op': 'in', - 'values': [ 'userkey' ] - } - ], - 'variation': 2 - } - ], + 'targets': [{ 'values': ['whoever', 'userkey'], 'variation': 2 }], 'fallthrough': { 'variation': 0 }, 'offVariation': 1, 'variations': ['a', 'b', 'c'] } user = { 'key': 'userkey' } - assert evaluate(flag, user, empty_store) == EvalResult(2, 'c', []) + detail = EvaluationDetail('c', 2, {'kind': 'TARGET_MATCH'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_matches_user_from_rules(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}], 'variation': 1} + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(True, 1, {'kind': 'RULE_MATCH', 'ruleIndex': 0, 'ruleId': 'id'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_rule_variation_is_too_high(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}], 'variation': 999} + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_rule_variation_is_negative(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}], 'variation': -1} + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_rule_has_no_variation_or_rollout(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}]} + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) + +def test_flag_returns_error_if_rule_has_rollout_with_no_variations(): + rule = { 'id': 'id', 'clauses': [{'attribute': 'key', 'op': 'in', 'values': ['userkey']}], + 'rollout': {'variations': []} } + flag = make_boolean_flag_with_rules([rule]) + user = { 'key': 'userkey' } + detail = EvaluationDetail(None, None, {'kind': 'ERROR', 'errorKind': 'MALFORMED_FLAG'}) + assert evaluate(flag, user, empty_store) == EvalResult(detail, []) def test_segment_match_clause_retrieves_segment_from_store(): store = InMemoryFeatureStore() @@ -154,7 +277,7 @@ def test_segment_match_clause_retrieves_segment_from_store(): ] } - assert evaluate(flag, user, store) == EvalResult(1, True, []) + assert evaluate(flag, user, store).detail.value == True def test_segment_match_clause_falls_through_with_no_errors_if_segment_not_found(): user = { "key": "foo" } @@ -177,7 +300,7 @@ def test_segment_match_clause_falls_through_with_no_errors_if_segment_not_found( ] } - assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) + assert evaluate(flag, user, empty_store).detail.value == False def test_clause_matches_builtin_attribute(): clause = { @@ -187,7 +310,7 @@ def test_clause_matches_builtin_attribute(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == EvalResult(1, True, []) + assert evaluate(flag, user, empty_store).detail.value == True def test_clause_matches_custom_attribute(): clause = { @@ -197,7 +320,7 @@ def test_clause_matches_custom_attribute(): } user = { 'key': 'x', 'name': 'Bob', 'custom': { 'legs': 4 } } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == EvalResult(1, True, []) + assert evaluate(flag, user, empty_store).detail.value == True def test_clause_returns_false_for_missing_attribute(): clause = { @@ -207,7 +330,7 @@ def test_clause_returns_false_for_missing_attribute(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) + assert evaluate(flag, user, empty_store).detail.value == False def test_clause_can_be_negated(): clause = { @@ -218,7 +341,7 @@ def test_clause_can_be_negated(): } user = { 'key': 'x', 'name': 'Bob' } flag = _make_bool_flag_from_clause(clause) - assert evaluate(flag, user, empty_store) == EvalResult(0, False, []) + assert evaluate(flag, user, empty_store).detail.value == False def _make_bool_flag_from_clause(clause): diff --git a/testing/test_flags_state.py b/testing/test_flags_state.py index c948dd3d..2fe5b123 100644 --- a/testing/test_flags_state.py +++ b/testing/test_flags_state.py @@ -6,7 +6,7 @@ def test_can_get_flag_value(): state = FeatureFlagsState(True) flag = { 'key': 'key' } - state.add_flag(flag, 'value', 1) + state.add_flag(flag, 'value', 1, None) assert state.get_flag_value('key') == 'value' def test_returns_none_for_unknown_flag(): @@ -17,16 +17,16 @@ def test_can_convert_to_values_map(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1' } flag2 = { 'key': 'key2' } - state.add_flag(flag1, 'value1', 0) - state.add_flag(flag2, 'value2', 1) + state.add_flag(flag1, 'value1', 0, None) + state.add_flag(flag2, 'value2', 1, None) assert state.to_values_map() == { 'key1': 'value1', 'key2': 'value2' } def test_can_convert_to_json_dict(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } - state.add_flag(flag1, 'value1', 0) - state.add_flag(flag2, 'value2', 1) + state.add_flag(flag1, 'value1', 0, None) + state.add_flag(flag2, 'value2', 1, None) result = state.to_json_dict() assert result == { @@ -52,8 +52,8 @@ def test_can_convert_to_json_string(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } - state.add_flag(flag1, 'value1', 0) - state.add_flag(flag2, 'value2', 1) + state.add_flag(flag1, 'value1', 0, None) + state.add_flag(flag2, 'value2', 1, None) obj = state.to_json_dict() str = state.to_json_string() @@ -63,8 +63,8 @@ def test_can_serialize_with_jsonpickle(): state = FeatureFlagsState(True) flag1 = { 'key': 'key1', 'version': 100, 'offVariation': 0, 'variations': [ 'value1' ], 'trackEvents': False } flag2 = { 'key': 'key2', 'version': 200, 'offVariation': 1, 'variations': [ 'x', 'value2' ], 'trackEvents': True, 'debugEventsUntilDate': 1000 } - state.add_flag(flag1, 'value1', 0) - state.add_flag(flag2, 'value2', 1) + state.add_flag(flag1, 'value1', 0, None) + state.add_flag(flag2, 'value2', 1, None) obj = state.to_json_dict() str = jsonpickle.encode(state, unpicklable=False) diff --git a/testing/test_ldclient.py b/testing/test_ldclient.py index db13a154..1766386b 100644 --- a/testing/test_ldclient.py +++ b/testing/test_ldclient.py @@ -58,6 +58,17 @@ def make_client(store): feature_store=store)) +def make_off_flag_with_value(key, value): + return { + u'key': key, + u'version': 100, + u'salt': u'', + u'on': False, + u'variations': [value], + u'offVariation': 0 + } + + def get_first_event(c): return c._event_processor._events.pop(0) @@ -149,93 +160,100 @@ def test_no_defaults(): def test_event_for_existing_feature(): - feature = { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': ['a', 'b'], - u'fallthrough': { - u'variation': 1 - }, - u'trackEvents': True - } + feature = make_off_flag_with_value('feature.key', 'value') + feature['trackEvents'] = True + feature['debugEventsUntilDate'] = 1000 store = InMemoryFeatureStore() store.init({FEATURES: {'feature.key': feature}}) client = make_client(store) - assert 'b' == client.variation('feature.key', user, default='c') + assert 'value' == client.variation('feature.key', user, default='default') e = get_first_event(client) assert (e['kind'] == 'feature' and e['key'] == 'feature.key' and e['user'] == user and - e['value'] == 'b' and - e['variation'] == 1 and - e['default'] == 'c' and - e['trackEvents'] == True) + e['version'] == feature['version'] and + e['value'] == 'value' and + e['variation'] == 0 and + e.get('reason') is None and + e['default'] == 'default' and + e['trackEvents'] == True and + e['debugEventsUntilDate'] == 1000) + + +def test_event_for_existing_feature_with_reason(): + feature = make_off_flag_with_value('feature.key', 'value') + feature['trackEvents'] = True + feature['debugEventsUntilDate'] = 1000 + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'value' == client.variation_detail('feature.key', user, default='default').value + e = get_first_event(client) + assert (e['kind'] == 'feature' and + e['key'] == 'feature.key' and + e['user'] == user and + e['version'] == feature['version'] and + e['value'] == 'value' and + e['variation'] == 0 and + e['reason'] == {'kind': 'OFF'} and + e['default'] == 'default' and + e['trackEvents'] == True and + e['debugEventsUntilDate'] == 1000) def test_event_for_unknown_feature(): store = InMemoryFeatureStore() store.init({FEATURES: {}}) client = make_client(store) - assert 'c' == client.variation('feature.key', user, default='c') + assert 'default' == client.variation('feature.key', user, default='default') e = get_first_event(client) assert (e['kind'] == 'feature' and e['key'] == 'feature.key' and e['user'] == user and - e['value'] == 'c' and + e['value'] == 'default' and e['variation'] == None and - e['default'] == 'c') + e['default'] == 'default') def test_event_for_existing_feature_with_no_user(): - feature = { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': ['a', 'b'], - u'fallthrough': { - u'variation': 1 - }, - u'trackEvents': True - } + feature = make_off_flag_with_value('feature.key', 'value') + feature['trackEvents'] = True + feature['debugEventsUntilDate'] = 1000 store = InMemoryFeatureStore() store.init({FEATURES: {'feature.key': feature}}) client = make_client(store) - assert 'c' == client.variation('feature.key', None, default='c') + assert 'default' == client.variation('feature.key', None, default='default') e = get_first_event(client) assert (e['kind'] == 'feature' and e['key'] == 'feature.key' and e['user'] == None and - e['value'] == 'c' and + e['version'] == feature['version'] and + e['value'] == 'default' and e['variation'] == None and - e['default'] == 'c' and - e['trackEvents'] == True) + e['default'] == 'default' and + e['trackEvents'] == True and + e['debugEventsUntilDate'] == 1000) def test_event_for_existing_feature_with_no_user_key(): - feature = { - u'key': u'feature.key', - u'salt': u'abc', - u'on': True, - u'variations': ['a', 'b'], - u'fallthrough': { - u'variation': 1 - }, - u'trackEvents': True - } + feature = make_off_flag_with_value('feature.key', 'value') + feature['trackEvents'] = True + feature['debugEventsUntilDate'] = 1000 store = InMemoryFeatureStore() store.init({FEATURES: {'feature.key': feature}}) client = make_client(store) bad_user = { u'name': u'Bob' } - assert 'c' == client.variation('feature.key', bad_user, default='c') + assert 'default' == client.variation('feature.key', bad_user, default='default') e = get_first_event(client) assert (e['kind'] == 'feature' and e['key'] == 'feature.key' and e['user'] == bad_user and - e['value'] == 'c' and + e['version'] == feature['version'] and + e['value'] == 'default' and e['variation'] == None and - e['default'] == 'c' and - e['trackEvents'] == True) + e['default'] == 'default' and + e['trackEvents'] == True and + e['debugEventsUntilDate'] == 1000) def test_secure_mode_hash(): diff --git a/testing/test_ldclient_evaluation.py b/testing/test_ldclient_evaluation.py index be8c74c5..9183034b 100644 --- a/testing/test_ldclient_evaluation.py +++ b/testing/test_ldclient_evaluation.py @@ -2,8 +2,10 @@ import json from ldclient.client import LDClient, Config from ldclient.feature_store import InMemoryFeatureStore +from ldclient.flag import EvaluationDetail from ldclient.versioned_data_kind import FEATURES from testing.stub_util import MockEventProcessor, MockUpdateProcessor +from testing.test_ldclient import make_off_flag_with_value user = { 'key': 'userkey' } @@ -32,6 +34,87 @@ def make_client(store): update_processor_class=MockUpdateProcessor, feature_store=store)) +def test_variation_for_existing_feature(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'value' == client.variation('feature.key', user, default='default') + +def test_variation_for_unknown_feature(): + store = InMemoryFeatureStore() + client = make_client(store) + assert 'default' == client.variation('feature.key', user, default='default') + +def test_variation_when_user_is_none(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'default' == client.variation('feature.key', None, default='default') + +def test_variation_when_user_has_no_key(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + assert 'default' == client.variation('feature.key', { }, default='default') + +def test_variation_for_flag_that_evaluates_to_none(): + empty_flag = { + 'key': 'feature.key', + 'on': False, + 'offVariation': None + } + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': empty_flag}}) + client = make_client(store) + assert 'default' == client.variation('feature.key', user, default='default') + +def test_variation_detail_for_existing_feature(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + expected = EvaluationDetail('value', 0, {'kind': 'OFF'}) + assert expected == client.variation_detail('feature.key', user, default='default') + +def test_variation_detail_for_unknown_feature(): + store = InMemoryFeatureStore() + client = make_client(store) + expected = EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'FLAG_NOT_FOUND'}) + assert expected == client.variation_detail('feature.key', user, default='default') + +def test_variation_detail_when_user_is_none(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + expected = EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'USER_NOT_SPECIFIED'}) + assert expected == client.variation_detail('feature.key', None, default='default') + +def test_variation_when_user_has_no_key(): + feature = make_off_flag_with_value('feature.key', 'value') + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': feature}}) + client = make_client(store) + expected = EvaluationDetail('default', None, {'kind': 'ERROR', 'errorKind': 'USER_NOT_SPECIFIED'}) + assert expected == client.variation_detail('feature.key', { }, default='default') + +def test_variation_detail_for_flag_that_evaluates_to_none(): + empty_flag = { + 'key': 'feature.key', + 'on': False, + 'offVariation': None + } + store = InMemoryFeatureStore() + store.init({FEATURES: {'feature.key': empty_flag}}) + client = make_client(store) + expected = EvaluationDetail('default', None, {'kind': 'OFF'}) + actual = client.variation_detail('feature.key', user, default='default') + assert expected == actual + assert actual.is_default_value() == True + def test_all_flags_returns_values(): store = InMemoryFeatureStore() store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) @@ -79,6 +162,34 @@ def test_all_flags_state_returns_state(): '$valid': True } +def test_all_flags_state_returns_state_with_reasons(): + store = InMemoryFeatureStore() + store.init({ FEATURES: { 'key1': flag1, 'key2': flag2 } }) + client = make_client(store) + state = client.all_flags_state(user, with_reasons=True) + assert state.valid == True + result = state.to_json_dict() + assert result == { + 'key1': 'value1', + 'key2': 'value2', + '$flagsState': { + 'key1': { + 'variation': 0, + 'version': 100, + 'trackEvents': False, + 'reason': {'kind': 'OFF'} + }, + 'key2': { + 'variation': 1, + 'version': 200, + 'trackEvents': True, + 'debugEventsUntilDate': 1000, + 'reason': {'kind': 'OFF'} + } + }, + '$valid': True + } + def test_all_flags_state_can_be_filtered_for_client_side_flags(): flag1 = { 'key': 'server-side-1',