Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: targeting key serialization fix #46

Merged
merged 2 commits into from
Feb 15, 2024
Merged

fix: targeting key serialization fix #46

merged 2 commits into from
Feb 15, 2024

Conversation

fabriziodemaria
Copy link
Member

The GO SDK flattens the context using targetingKey as key for the targeting key value in the context.

Other SDK pass the (non-flattened) EvaluationContext object to the Provider and let the latter decide how to encode the context. We might want to investigate more why the implementation in GO differs in these regards.

In the meantime, this fix allows to keep backwards compatibility and inter-operability between different backends at the cost of sending duplicate data. Note that the user can always set targeting_key as an attribute

Comment on lines +99 to +106
func processTargetingKey(evalCtx openfeature.FlattenedContext) openfeature.FlattenedContext {
newEvalContext := openfeature.FlattenedContext{}
newEvalContext = evalCtx
if targetingKey, exists := evalCtx["targetingKey"]; exists {
newEvalContext["targeting_key"] = targetingKey
}
return newEvalContext
}
Copy link
Member

Choose a reason for hiding this comment

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

Can't say much about the Go specifics but looks to do the right thing.
Would love to see a unit test for this!

@fabriziodemaria fabriziodemaria merged commit 859dd88 into main Feb 15, 2024
2 checks passed
@fabriziodemaria fabriziodemaria deleted the targeting_key branch February 15, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants