Skip to content

Commit d4dcdb3

Browse files
fix: Keep a multi-context kind whose targetingKey is not a string (#77)
**Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Related issues** OpenFeature provider spec (`OFP`) Requirements 2.2.1 and 2.3.x in `launchdarkly/sdk-specs`. **Describe the solution you've provided** In a multi-context, if an individual context had a `targetingKey` that was not a string, the converter silently dropped that whole kind with no log. For example, `{'kind': 'multi', 'user': {'targetingKey': False, 'key': 'user-key'}, 'org': {...}}` produced only the `org` context. OFP 2.2.1 requires each top-level attribute to become a single context. Section 2.3 applies the key rules within each individual context, using its `targetingKey` in place of the OpenFeature targeting key. Under those rules, a non-string `targetingKey` is not usable, so the provider falls back to a string `key`. If there is no key, the provider produces an invalid context and logs an error (2.3.5), rather than dropping the kind. The converter now treats a non-string nested `targetingKey` as absent and logs a warning, matching how a non-string `key` is handled (2.3.4.1), then applies the normal key rules. `test_create_multi_context_with_invalid_targeting_key` asserted the old dropping behavior, so it is updated. A new test covers the case with no fallback key. **Describe alternatives you've considered** Keeping the drop and only adding a log. That would still lose targeting for a kind that has a valid `key`, which the spec does not allow. **Additional context** This changes behavior only for the malformed input above. Found during the weekly OpenFeature provider audit. Link to Devin session: https://app.devin.ai/sessions/f7bc946afe8c41a28cf7f3f497fc8b9d Open in Devin Desktop: https://app.devin.ai/desktop/session/f7bc946afe8c41a28cf7f3f497fc8b9d?variant=devin [SDK-3496](https://launchdarkly.atlassian.net/browse/SDK-3496) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Fixes **multi-context conversion** when an individual kind has a non-string `targetingKey`. Previously that kind was **silently omitted** from the built context; it now **logs a warning**, treats `targetingKey` as unset (same pattern as a bad `key`), and continues with normal key resolution via the string `key` when present. > > **Tests** now expect a full multi-context (e.g. `user` kept with `key` `'user-key'`) plus the warning, and add coverage where invalid `targetingKey` and no string `key` make the overall context **invalid** instead of dropping only that kind. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4be37ad. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> [SDK-3496]: https://launchdarkly.atlassian.net/browse/SDK-3496?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent a6f8092 commit d4dcdb3

2 files changed

Lines changed: 28 additions & 4 deletions

File tree

‎ld_openfeature/impl/context_converter.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ def __build_multi_context(self, context: EvaluationContext) -> Context:
9797
targeting_key = attributes.get('targetingKey')
9898

9999
if targeting_key is not None and not isinstance(targeting_key, str):
100-
continue
100+
logger.warning("A non-string 'targetingKey' attribute was provided.")
101+
targeting_key = None
101102

102103
targeting_key = self.__get_targeting_key(targeting_key, key)
103104
single_context = self.__build_single_context(attributes, kind, targeting_key)

‎tests/impl/test_context_converter.py‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def test_invalid_private_attribute_types_are_ignored(context_converter: Evaluati
5252
assert caplog.records[0].message == "'privateAttributes' must be an array of only string values"
5353

5454

55-
def test_create_multi_context_with_invalid_targeting_key(context_converter: EvaluationContextConverter):
55+
def test_create_multi_context_with_invalid_targeting_key(context_converter: EvaluationContextConverter, caplog):
5656
attributes: Mapping[str, EvaluationContextAttribute] = {
5757
'kind': 'multi',
5858
'user': {'targetingKey': False, 'key': 'user-key', 'name': 'User name'},
@@ -62,8 +62,31 @@ def test_create_multi_context_with_invalid_targeting_key(context_converter: Eval
6262
ld_context = context_converter.to_ld_context(context)
6363

6464
assert ld_context.valid is True
65-
assert ld_context.key == 'org-key'
66-
assert ld_context.kind == 'org'
65+
assert ld_context.multiple is True
66+
user_context = ld_context.get_individual_context('user')
67+
assert user_context is not None
68+
assert user_context.key == 'user-key'
69+
assert user_context.name == 'User name'
70+
assert user_context.get('targetingKey') is None
71+
org_context = ld_context.get_individual_context('org')
72+
assert org_context is not None
73+
assert org_context.key == 'org-key'
74+
75+
assert len(caplog.records) == 1
76+
assert caplog.records[0].message == "A non-string 'targetingKey' attribute was provided."
77+
assert caplog.records[0].levelname == 'WARNING'
78+
79+
80+
def test_multi_context_with_invalid_targeting_key_and_no_key_is_invalid(context_converter: EvaluationContextConverter):
81+
attributes: Mapping[str, EvaluationContextAttribute] = {
82+
'kind': 'multi',
83+
'user': {'targetingKey': False, 'name': 'User name'},
84+
'org': {'key': 'org-key', 'name': 'Org name'},
85+
}
86+
context = EvaluationContext(None, attributes)
87+
ld_context = context_converter.to_ld_context(context)
88+
89+
assert ld_context.valid is False
6790

6891

6992
def test_create_context_with_only_key(context_converter: EvaluationContextConverter):

0 commit comments

Comments
 (0)