-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat: add variant strategy support #135
Changes from 6 commits
7fcf526
c6db0eb
590fc1c
90787c9
caea56f
37e6297
c914ced
03914f5
7c65044
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -243,12 +243,12 @@ func (uc *Client) IsEnabled(feature string, options ...FeatureOption) (enabled b | |
uc.metrics.count(feature, enabled) | ||
}() | ||
|
||
return uc.isEnabled(feature, options...) | ||
return uc.isEnabled(feature, options...).Enabled | ||
} | ||
|
||
// isEnabled abstracts away the details of checking if a toggle is turned on or off | ||
// without metrics | ||
func (uc *Client) isEnabled(feature string, options ...FeatureOption) (enabled bool) { | ||
func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.StrategyResult { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of simple boolean, we are now returning complex object, that includes variant. |
||
var opts featureOption | ||
for _, o := range options { | ||
o(&opts) | ||
|
@@ -268,19 +268,29 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) (enabled b | |
|
||
if f == nil { | ||
if opts.fallbackFunc != nil { | ||
return opts.fallbackFunc(feature, ctx) | ||
return api.StrategyResult{ | ||
Enabled: opts.fallbackFunc(feature, ctx), | ||
} | ||
} else if opts.fallback != nil { | ||
return *opts.fallback | ||
return api.StrategyResult{ | ||
Enabled: *opts.fallback, | ||
} | ||
} | ||
return api.StrategyResult{ | ||
Enabled: false, | ||
} | ||
return false | ||
} | ||
|
||
if !f.Enabled { | ||
return false | ||
return api.StrategyResult{ | ||
Enabled: false, | ||
} | ||
} | ||
|
||
if len(f.Strategies) == 0 { | ||
return f.Enabled | ||
return api.StrategyResult{ | ||
Enabled: f.Enabled, | ||
} | ||
} | ||
|
||
for _, s := range f.Strategies { | ||
|
@@ -294,7 +304,9 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) (enabled b | |
|
||
if err != nil { | ||
uc.errors <- err | ||
return false | ||
return api.StrategyResult{ | ||
Enabled: false, | ||
} | ||
} | ||
|
||
allConstraints := make([]api.Constraint, 0) | ||
|
@@ -304,11 +316,25 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) (enabled b | |
if ok, err := constraints.Check(ctx, allConstraints); err != nil { | ||
uc.errors <- err | ||
} else if ok && foundStrategy.IsEnabled(s.Parameters, ctx) { | ||
return true | ||
if s.Variants != nil && len(s.Variants) > 0 { | ||
return api.StrategyResult{ | ||
Enabled: true, | ||
Variant: api.VariantSetup{ | ||
Name: f.Name, | ||
Variants: s.Variants, | ||
}.GetVariant(ctx), | ||
} | ||
} else { | ||
return api.StrategyResult{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If strategy does not have variants, just return true and use old variants. |
||
Enabled: true, | ||
} | ||
} | ||
} | ||
} | ||
|
||
return false | ||
return api.StrategyResult{ | ||
Enabled: false, | ||
} | ||
} | ||
|
||
// GetVariant queries a variant as the specified feature is enabled. | ||
|
@@ -335,14 +361,16 @@ func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOpt | |
ctx = ctx.Override(*opts.ctx) | ||
} | ||
|
||
var strategyResult api.StrategyResult | ||
|
||
if opts.resolver != nil { | ||
if !uc.isEnabled(feature, WithContext(*ctx), WithResolver(opts.resolver)) { | ||
return defaultVariant | ||
} | ||
strategyResult = uc.isEnabled(feature, WithContext(*ctx), WithResolver(opts.resolver)) | ||
} else { | ||
if !uc.isEnabled(feature, WithContext(*ctx)) { | ||
return defaultVariant | ||
} | ||
strategyResult = uc.isEnabled(feature, WithContext(*ctx)) | ||
} | ||
|
||
if !strategyResult.Enabled { | ||
return defaultVariant | ||
} | ||
|
||
var f *api.Feature | ||
|
@@ -365,11 +393,18 @@ func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOpt | |
return defaultVariant | ||
} | ||
|
||
if strategyResult.Variant != nil { | ||
return strategyResult.Variant | ||
} | ||
|
||
if len(f.Variants) == 0 { | ||
return defaultVariant | ||
} | ||
|
||
return f.GetVariant(ctx) | ||
return api.VariantSetup{ | ||
Name: f.Name, | ||
Variants: f.Variants, | ||
}.GetVariant(ctx) | ||
} | ||
|
||
// Close stops the client from syncing data from the server. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update client spec to latest