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

Feat/dependent feature toggles #140

Merged
merged 13 commits into from
Sep 27, 2023
13 changes: 13 additions & 0 deletions api/feature.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ type Feature struct {

// Variants is a list of variants of the feature toggle.
Variants []VariantInternal `json:"variants"`

// Dependencies is a list of feature toggle dependency objects
Dependencies *[]Dependency `json:"dependencies"`
}

type Dependency struct {
// Feature is the name of the feature toggle we depend upon
Feature string `json:"feature"`
// Variants contains a string of variants that the dependency should resolve to
Variants *[]string `json:"variants"`
// Enabled is the property that determines whether the dependency should be on or off
// If the property is absent from the payload it's assumed to be default on
Enabled *bool `json:"enabled"`
}

func (fr FeatureResponse) FeatureMap() map[string]interface{} {
Expand Down
39 changes: 19 additions & 20 deletions api/feature_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,29 @@ import (
func TestFeatureSegmentsResponse(t *testing.T) {
assert := assert.New(t)
featureResponse := FeatureResponse{
Features: []Feature{},
Segments: []Segment{
{Id: 1, Constraints: []Constraint{
{
ContextName: "custom-id",
Operator: OperatorIn,
Values: []string{"custom-ctx"},
}}},
{Id: 2, Constraints: []Constraint{
{
ContextName: "age",
Operator: OperatorNumGte,
Value: "5",
}}},
}}
Features: []Feature{},
Segments: []Segment{
{Id: 1, Constraints: []Constraint{
{
ContextName: "custom-id",
Operator: OperatorIn,
Values: []string{"custom-ctx"},
}}},
{Id: 2, Constraints: []Constraint{
{
ContextName: "age",
Operator: OperatorNumGte,
Value: "5",
}}},
}}

segmentsMap := featureResponse.SegmentsMap();
segmentsMap := featureResponse.SegmentsMap()

segmentOne := segmentsMap[1];
segmentTwo := segmentsMap[2];
segmentOne := segmentsMap[1]
segmentTwo := segmentsMap[2]
segmentThree := segmentsMap[3]

assert.Equal(segmentOne[0].Operator, OperatorIn)
assert.Equal(segmentTwo[0].Operator, OperatorNumGte)
assert.Nil(segmentThree)
}

89 changes: 74 additions & 15 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,31 +254,25 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate
o(&opts)
}

var f *api.Feature
if opts.resolver != nil {
f = opts.resolver(feature)
} else {
f = uc.repository.getToggle(feature)
}
f := resolveToggle(uc, opts, feature)

ctx := uc.staticContext
if opts.ctx != nil {
ctx = ctx.Override(*opts.ctx)
}

if f == nil {
if opts.fallbackFunc != nil {
return api.StrategyResult{
Enabled: opts.fallbackFunc(feature, ctx),
}
} else if opts.fallback != nil {
return handleFallback(opts, feature, ctx)
}

if f.Dependencies != nil && len(*f.Dependencies) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why you decided to do this check outside the isParentDependencySatisfied. I made the opposite decision but didn't have a strong opinion here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To me it made more sense readability wise. If I don't have dependencies or the length of dependencies is 0 then I don't need to do check if isParentDependencySatisfied(f, *ctx). Moving it inside that function obfuscates the check that determines whether or not that function should actually be run. I don't have strong opinions about this either, but I found it easier to reason about this way because now the function is limited to doing one thing and one thing only.

dependenciesSatisfied := uc.isParentDependencySatisfied(f, *ctx)

if !dependenciesSatisfied {
return api.StrategyResult{
Enabled: *opts.fallback,
Enabled: false,
}
}
return api.StrategyResult{
Enabled: false,
}
}

if !f.Enabled {
Expand Down Expand Up @@ -345,6 +339,44 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate
}
}

func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context context.Context) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context context.Context) bool {
    if feature == nil || feature.Dependencies == nil || len(*feature.Dependencies) == 0 {
        return true
    }

    for _, parent := range *feature.Dependencies {
        parentToggle := uc.repository.getToggle(parent.Feature)
        isEnabled := uc.isEnabled(parent.Feature, WithContext(context)).Enabled

        if parentToggle == nil || 
           (parentToggle.Dependencies != nil && len(*parentToggle.Dependencies) > 0) || 
           (parent.Enabled != nil && !isEnabled) {
            return false
        }

        if parent.Enabled == nil {
            variantName := uc.getVariantWithoutMetrics(parent.Feature, WithVariantContext(context)).Name
            hasValidVariant := parent.Variants != nil && len(*parent.Variants) > 0 && contains(*parent.Variants, variantName)
            
            if !hasValidVariant && !isEnabled {
                return false
            }
        }
    }

    return true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unless we write an every helper, we can't return out here or we will exit the entire function without properly looping over all the dependencies.

warnOnce := &WarnOnce{}

dependenciesSatisfied := func(parent api.Dependency) bool {
parentToggle := uc.repository.getToggle(parent.Feature)

if parentToggle == nil {
warnOnce.Warn("the parent toggle was not found in the cache, the evaluation of this dependency will always be false")
return false
}

if parentToggle.Dependencies != nil && len(*parentToggle.Dependencies) > 0 {
return false
}

// According to the schema, if the enabled property is absent we assume it's true.
if parent.Enabled == nil {
if parent.Variants != nil && len(*parent.Variants) > 0 {
variantName := uc.getVariantWithoutMetrics(parent.Feature, WithVariantContext(context)).Name
return contains(*parent.Variants, variantName)
}
return uc.isEnabled(parent.Feature, WithContext(context)).Enabled
}

return !uc.isEnabled(parent.Feature, WithContext(context)).Enabled
}

allDependenciesSatisfied := every(*feature.Dependencies, func(parent interface{}) bool {
return dependenciesSatisfied(parent.(api.Dependency))
})

if !allDependenciesSatisfied {
return false
}

return true
}

// GetVariant queries a variant as the specified feature is enabled.
//
// It is safe to call this method from multiple goroutines concurrently.
Expand Down Expand Up @@ -483,3 +515,30 @@ func (uc *Client) WaitForReady() {
func (uc *Client) ListFeatures() []api.Feature {
return uc.repository.list()
}

func resolveToggle(unleashClient *Client, opts featureOption, featureName string) *api.Feature {
var feature *api.Feature
if opts.resolver != nil {
feature = opts.resolver(featureName)
} else {
feature = unleashClient.repository.getToggle(featureName)
}

return feature
}

func handleFallback(opts featureOption, featureName string, ctx *context.Context) api.StrategyResult {
if opts.fallbackFunc != nil {
return api.StrategyResult{
Enabled: opts.fallbackFunc(featureName, ctx),
}
} else if opts.fallback != nil {
return api.StrategyResult{
Enabled: *opts.fallback,
}
}

return api.StrategyResult{
Enabled: false,
}
}
16 changes: 8 additions & 8 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Package unleash is a client library for connecting to an Unleash feature toggle

See https://github.com/Unleash/unleash for more information.

Basics
# Basics

The API is very simple. The main functions of interest are Initialize and IsEnabled. Calling Initialize
will create a default client and if a listener is supplied, it will start the sync loop. Internally the client
Expand All @@ -17,27 +17,27 @@ creates a set of channels that it passes to both of the above components and it
asynchronously. It is important to ensure that these channels get regularly drained to avoid blocking those
Go routines. There are two ways this can be done.

Using the Listener Interfaces
# Using the Listener Interfaces

The first and perhaps simplest way to "drive" the synchronization loop in the client is to provide a type
that implements one or more of the listener interfaces. There are 3 interfaces and you can choose which ones
you should implement:
- ErrorListener
- RepositoryListener
- MetricsListener
- ErrorListener
- RepositoryListener
- MetricsListener

If you are only interesting in tracking errors and warnings and don't care about any of the other signals,
then you only need to implement the ErrorListener and pass this instance to WithListener(). The DebugListener
shows an example of implementing all of the listeners in a single type.

Reading the channels directly
# Reading the channels directly

If you would prefer to have control over draining the channels yourself, then you must not call WithListener().
Instead, you should read all of the channels continuously inside a select. The WithInstance example shows how
to do this. Note that all channels must be drained, even if you are not interested in the result.

Examples
# Examples

The following examples show how to use the client in different scenarios.

*/
package unleash
77 changes: 77 additions & 0 deletions metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,80 @@ func TestMetrics_SendMetricsFail(t *testing.T) {
// Now OnSent should have been called as /client/metrics returned 200.
mockListener.AssertCalled(t, "OnSent", mock.AnythingOfType("MetricsData"))
}

func TestMetrics_ShouldNotCountMetricsForParentToggles(t *testing.T) {
assert := assert.New(t)
defer gock.OffAll()

gock.New(mockerServer).
Post("/client/register").
Reply(200)

gock.New(mockerServer).
Get("/client/features").
Reply(200).
JSON(api.FeatureResponse{
Features: []api.Feature{
{
Name: "parent",
Enabled: true,
Description: "parent toggle",
Strategies: []api.Strategy{
{
Id: 1,
Name: "flexibleRollout",
Constraints: []api.Constraint{},
Parameters: map[string]interface{}{
"rollout": 100,
"stickiness": "default",
},
},
},
},
{
Name: "child",
Enabled: true,
Description: "parent toggle",
Strategies: []api.Strategy{
{
Id: 1,
Name: "flexibleRollout",
Constraints: []api.Constraint{},
Parameters: map[string]interface{}{
"rollout": 100,
"stickiness": "default",
},
},
},
Dependencies: &[]api.Dependency{
{
Feature: "parent",
},
},
},
},
})

mockListener := &MockedListener{}
mockListener.On("OnReady").Return()
mockListener.On("OnError").Return()
mockListener.On("OnRegistered", mock.AnythingOfType("ClientData"))
mockListener.On("OnCount", "child", true).Return()

client, err := NewClient(
WithUrl(mockerServer),
WithAppName(mockAppName),
WithInstanceId(mockInstanceId),
WithListener(mockListener),
)

client.WaitForReady()
client.IsEnabled("child")

assert.EqualValues(client.metrics.bucket.Toggles["child"].Yes, 1)
assert.EqualValues(client.metrics.bucket.Toggles["parent"].Yes, 0)
client.Close()

assert.Nil(err, "client should not return an error")
assert.True(gock.IsDone(), "there should be no more mocks")
}
4 changes: 2 additions & 2 deletions repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func TestRepository_ParseAPIResponse(t *testing.T) {
}
}`)

reader := bytes.NewReader(data);
reader := bytes.NewReader(data)
dec := json.NewDecoder(reader)

var response api.FeatureResponse
Expand All @@ -126,4 +126,4 @@ func TestRepository_ParseAPIResponse(t *testing.T) {

assert.Equal(2, len(response.Features))
assert.Equal(0, len(response.Segments))
}
}
44 changes: 44 additions & 0 deletions utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"math/rand"
"os"
"os/user"
"reflect"
"sync"
"time"
)

Expand Down Expand Up @@ -34,3 +36,45 @@ func getFetchURLPath(projectName string) string {
}
return "./client/features"
}

func contains(arr []string, str string) bool {
for _, item := range arr {
if item == str {
return true
}
}
return false
}

// WarnOnce is a type for handling warnings that should only be displayed once.
type WarnOnce struct {
once sync.Once
}

// Warn logs the warning message once.
func (wo *WarnOnce) Warn(message string) {
wo.once.Do(func() {
fmt.Println("Warning:", message)
})
}

func every(slice interface{}, condition func(interface{}) bool) bool {
sliceValue := reflect.ValueOf(slice)

if sliceValue.Kind() != reflect.Slice {
fmt.Println("Input is not a slice returning false")
return false
}

if (sliceValue.Len() == 0) {
return false
}

for i := 0; i < sliceValue.Len(); i++ {
element := sliceValue.Index(i).Interface()
if !condition(element) {
return false
}
}
return true
}
Loading
Loading