Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
[submodule "providers/openfeature-provider-flagd/openfeature/test-harness"]
path = providers/openfeature-provider-flagd/openfeature/test-harness
url = https://github.com/open-feature/flagd-testbed.git
branch = v2.8.0
branch = v2.10.2
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
)
from openfeature.evaluation_context import EvaluationContext
from openfeature.event import ProviderEventDetails
from openfeature.exception import FlagNotFoundError, ParseError
from openfeature.exception import ErrorCode, FlagNotFoundError, GeneralError, ParseError
from openfeature.flag_evaluation import FlagResolutionDetails, Reason

from ..config import Config
from .process.connector import FlagStateConnector
from .process.connector.grpc_watcher import GrpcWatcher
from .process.flags import FlagStore
from .process.flags import Flag, FlagStore
from .process.targeting import targeting

T = typing.TypeVar("T")
Expand Down Expand Up @@ -128,30 +128,55 @@ def _resolve(
)

if not flag.targeting:
variant, value = flag.default
return FlagResolutionDetails(
value, variant=variant, flag_metadata=metadata, reason=Reason.STATIC
)
return _default_resolve(flag, metadata, Reason.STATIC)

variant = targeting(flag.key, flag.targeting, evaluation_context)
try:
variant = targeting(flag.key, flag.targeting, evaluation_context)
if variant is None:
return _default_resolve(flag, metadata, Reason.DEFAULT)

if variant is None:
variant, value = flag.default
return FlagResolutionDetails(
value, variant=variant, flag_metadata=metadata, reason=Reason.DEFAULT
)
if not isinstance(variant, (str, bool)):
raise ParseError(
"Parsed JSONLogic targeting did not return a string or bool"
)
# convert to string to support shorthand (boolean in python is with capital T hence the special case)
if isinstance(variant, bool):
variant = str(variant).lower()
elif not isinstance(variant, str):
variant = str(variant)

if variant not in flag.variants:
raise GeneralError(
f"Resolved variant {variant} not in variants config."
)

except ReferenceError:
raise ParseError(f"Invalid targeting {targeting}") from ReferenceError

variant, value = flag.get_variant(variant)
if value is None:
raise ParseError(f"Resolved variant {variant} not in variants config.")
raise GeneralError(f"Resolved variant {variant} not in variants config.")

return FlagResolutionDetails(
value,
variant=variant,
reason=Reason.TARGETING_MATCH,
flag_metadata=metadata,
)


def _default_resolve(
flag: Flag,
metadata: typing.Mapping[str, typing.Union[float, int, str, bool]],
reason: Reason,
) -> FlagResolutionDetails:
variant, value = flag.default
if variant is None:
return FlagResolutionDetails(
value,
variant=variant,
reason=Reason.ERROR,
error_code=ErrorCode.FLAG_NOT_FOUND,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I compared this to the other methods, and there is a special error message used ""Flag '" + key+ "' has no default variant defined, will use code default"" maybe we should adapt the tests to add the message to be in sync in all of them? wdyt? @leakonvalinka

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it definitely wouldn't hurt
however i dont know if the error messages are actually used for anything except logs? if not, i think this doesn't have a high priority then

flag_metadata=metadata,
)
if variant not in flag.variants:
raise GeneralError(f"Resolved variant {variant} not in variants config.")
return FlagResolutionDetails(
value, variant=variant, flag_metadata=metadata, reason=reason
)
Original file line number Diff line number Diff line change
Expand Up @@ -72,30 +72,22 @@ class Flag:
key: str
state: str
variants: typing.Mapping[str, typing.Any]
default_variant: typing.Union[bool, str]
default_variant: typing.Optional[typing.Union[bool, str]] = None
targeting: typing.Optional[dict] = None
metadata: typing.Optional[
typing.Mapping[str, typing.Union[float, int, str, bool]]
] = None

def __post_init__(self) -> None:
if not self.state or not isinstance(self.state, str):
if not self.state or not (self.state == "ENABLED" or self.state == "DISABLED"):
raise ParseError("Incorrect 'state' value provided in flag config")

if not self.variants or not isinstance(self.variants, dict):
raise ParseError("Incorrect 'variants' value provided in flag config")

if not self.default_variant or not isinstance(
self.default_variant, (str, bool)
):
if self.default_variant and not isinstance(self.default_variant, (str, bool)):
raise ParseError("Incorrect 'defaultVariant' value provided in flag config")

if self.targeting and not isinstance(self.targeting, dict):
raise ParseError("Incorrect 'targeting' value provided in flag config")

if self.default_variant not in self.variants:
raise ParseError("Default variant does not match set of variants")

if self.metadata:
if not isinstance(self.metadata, dict):
raise ParseError("Flag metadata is not a valid json object")
Expand All @@ -106,6 +98,8 @@ def __post_init__(self) -> None:
def from_dict(cls, key: str, data: dict) -> "Flag":
if "defaultVariant" in data:
data["default_variant"] = data["defaultVariant"]
if data["default_variant"] == "":
data["default_variant"] = None
del data["defaultVariant"]

data.pop("source", None)
Expand All @@ -119,13 +113,16 @@ def from_dict(cls, key: str, data: dict) -> "Flag":
raise ParseError from err

@property
def default(self) -> tuple[str, typing.Any]:
def default(self) -> tuple[typing.Optional[str], typing.Any]:
return self.get_variant(self.default_variant)

def get_variant(
self, variant_key: typing.Union[str, bool]
) -> tuple[str, typing.Any]:
self, variant_key: typing.Union[str, bool, None]
) -> tuple[typing.Optional[str], typing.Any]:
if isinstance(variant_key, bool):
variant_key = str(variant_key).lower()

if not variant_key:
return None, None

return variant_key, self.variants.get(variant_key)
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from json_logic.types import JsonValue

from openfeature.evaluation_context import EvaluationContext
from openfeature.exception import ParseError

from .custom_ops import (
ends_with,
Expand All @@ -27,6 +28,9 @@ def targeting(
targeting: dict,
evaluation_context: typing.Optional[EvaluationContext] = None,
) -> JsonValue:
if not isinstance(targeting, dict):
raise ParseError(f"Invalid 'targeting' value in flag: {targeting}")

json_logic_context = evaluation_context.attributes if evaluation_context else {}
json_logic_context["$flagd"] = {"flagKey": key, "timestamp": int(time.time())}
json_logic_context["targetingKey"] = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def start(self) -> "FlagdContainer":
return self

@wait_container_is_ready(ConnectionError)
def _checker(self, host: str, port: str) -> None:
def _checker(self, host: str, port: int) -> None:
# First we wait for Flagd to say it's listening
wait_for_logs(
self,
Expand All @@ -58,7 +58,7 @@ def _checker(self, host: str, port: str) -> None:

time.sleep(1)
# Second we use the GRPC health check endpoint
with grpc.insecure_channel(host + ":" + port) as channel:
with grpc.insecure_channel(host + ":" + str(port)) as channel:
health_stub = health_pb2_grpc.HealthStub(channel)

def health_check_call(stub: health_pb2_grpc.HealthStub):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ def update_context(
evaluation_context.attributes[key] = type_cast[type_info](value)


@given(
parsers.cfparse(
'a context containing a key "{key}", with type "{type_info}" and with value ""'
),
)
def update_context_without_value(
evaluation_context: EvaluationContext, key: str, type_info: str
):
"""a context containing a key and value."""
update_context(evaluation_context, key, type_info, "")


@when(
parsers.cfparse(
'context contains keys {fields:s} with values "{svalue}", "{svalue2}", {ivalue:d}, "{bvalue:bool}"',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import requests
from asserts import assert_equal
from asserts import assert_equal, assert_is_none
from pytest_bdd import given, parsers, then, when

from openfeature.client import OpenFeatureClient
Expand Down Expand Up @@ -96,6 +96,25 @@ def resolve_details_reason(
assert_equal(details.reason, Reason(reason))


@then(
parsers.cfparse('the error-code should be "{error_code}"'),
)
def resolve_details_error_code(
details: FlagEvaluationDetails[JsonPrimitive],
error_code: str,
):
assert_equal(details.error_code, error_code)


@then(
parsers.cfparse('the error-code should be ""'),
)
def resolve_details_empty_error_code(
details: FlagEvaluationDetails[JsonPrimitive],
):
assert_is_none(details.error_code)


@then(parsers.cfparse("the resolved metadata should contain"))
def metadata_contains(details: FlagEvaluationDetails[JsonPrimitive], datatable):
assert_equal(len(details.flag_metadata), len(datatable) - 1) # skip table header
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"flags": {
"basic-flag": {
"state": "ENABLED",
"variants": {
"true": true,
"false": false
},
"targeting": {}
}
}
}
34 changes: 32 additions & 2 deletions providers/openfeature-provider-flagd/tests/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@ def create_client(provider: FlagdProvider):
"not-a-flag.json",
"basic-flag-wrong-structure.json",
"basic-flag-invalid.not-json",
"basic-flag-wrong-variant.json",
"basic-flag-broken-state.json",
"basic-flag-broken-variants.json",
"basic-flag-broken-default.json",
"basic-flag-broken-targeting.json",
],
)
def test_file_load_errors(file_name: str):
Expand All @@ -46,6 +44,38 @@ def test_file_load_errors(file_name: str):
assert res.error_code == ErrorCode.FLAG_NOT_FOUND


def test_non_existent_variant():
path = os.path.abspath(os.path.join(os.path.dirname(__file__), "./flags/"))
client = create_client(
FlagdProvider(
resolver_type=ResolverType.IN_PROCESS,
offline_flag_source_path=f"{path}/basic-flag-wrong-variant.json",
)
)

res = client.get_boolean_details("basic-flag", False)

assert res.value is False
assert res.reason == Reason.ERROR
assert res.error_code == ErrorCode.GENERAL


def test_broken_targeting():
path = os.path.abspath(os.path.join(os.path.dirname(__file__), "./flags/"))
client = create_client(
FlagdProvider(
resolver_type=ResolverType.IN_PROCESS,
offline_flag_source_path=f"{path}/basic-flag-broken-targeting.json",
)
)

res = client.get_boolean_details("basic-flag", False)

assert res.value is False
assert res.reason == Reason.ERROR
assert res.error_code == ErrorCode.PARSE_ERROR


@pytest.mark.parametrize(
"file_name",
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ def create_client(provider: FlagdProvider):
"file_name",
[
"basic-flag.json",
"basic-flag-without-default.json",
"basic-flag.yaml",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from openfeature.contrib.provider.flagd.resolvers.in_process import InProcessResolver
from openfeature.contrib.provider.flagd.resolvers.process.flags import Flag, FlagStore
from openfeature.evaluation_context import EvaluationContext
from openfeature.exception import FlagNotFoundError, ParseError
from openfeature.exception import FlagNotFoundError, GeneralError


def targeting():
Expand Down Expand Up @@ -79,7 +79,7 @@ def test_resolve_boolean_details_invalid_variant(resolver, flag):

resolver.flag_store.get_flag = Mock(return_value=flag)

with pytest.raises(ParseError):
with pytest.raises(GeneralError):
resolver.resolve_boolean_details("flag", False)


Expand Down