From 41102d83b87f29a7e50d400c1b8dfc02e25d66fe Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Tue, 26 Sep 2023 14:39:49 +0200 Subject: [PATCH 01/13] feat: dependable toggles --- api/feature.go | 12 +++++++++++ client.go | 56 +++++++++++++++++++++++++++++++++++++++++++++++++- utils.go | 9 ++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/api/feature.go b/api/feature.go index ccdc700..7f4fff4 100644 --- a/api/feature.go +++ b/api/feature.go @@ -47,8 +47,20 @@ 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 *[]FeatureDependencies `json:"dependencies"` } +type FeatureDependencies struct { + // The name of the feature toggle we depend upon + Feature string `json:"feature"` + // Possible support for depending on a variant in the future + Variants *[]string `json:"variants"` + + Enabled *bool `json:"enabled"` +} + func (fr FeatureResponse) FeatureMap() map[string]interface{} { features := map[string]interface{}{} for _, f := range fr.Features { diff --git a/client.go b/client.go index f725bd7..ffb3584 100644 --- a/client.go +++ b/client.go @@ -242,7 +242,7 @@ func (uc *Client) IsEnabled(feature string, options ...FeatureOption) (enabled b defer func() { uc.metrics.count(feature, enabled) }() - + return uc.isEnabled(feature, options...).Enabled } @@ -261,10 +261,12 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate f = uc.repository.getToggle(feature) } + ctx := uc.staticContext if opts.ctx != nil { ctx = ctx.Override(*opts.ctx) } + if f == nil { if opts.fallbackFunc != nil { @@ -281,6 +283,16 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate } } + if f.Dependencies != nil && len(*f.Dependencies) > 0 { + parentEnabled := uc.isParentDependencySatisfied(f, *ctx) + + if !parentEnabled { + return api.StrategyResult{ + Enabled: false, + } + } + } + if !f.Enabled { return api.StrategyResult{ Enabled: false, @@ -345,6 +357,48 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate } } +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) + + if parentToggle == nil { + 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 + if contains(*parent.Variants, variantName) { + continue + } + } else { + if uc.isEnabled(parent.Feature, WithContext(context)).Enabled { + continue + } + } + } else { + if !uc.isEnabled(parent.Feature, WithContext(context)).Enabled { + continue + } + } + + 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. diff --git a/utils.go b/utils.go index 4b2d1e8..0b12072 100644 --- a/utils.go +++ b/utils.go @@ -34,3 +34,12 @@ 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 +} \ No newline at end of file From b521b275a628101ebe0ee9f12aaef8b2eb155c2d Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Tue, 26 Sep 2023 14:48:56 +0200 Subject: [PATCH 02/13] fix: clean up isEnabled --- client.go | 57 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/client.go b/client.go index ffb3584..5ed410c 100644 --- a/client.go +++ b/client.go @@ -246,6 +246,7 @@ func (uc *Client) IsEnabled(feature string, options ...FeatureOption) (enabled b 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) api.StrategyResult { @@ -253,34 +254,16 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate for _, o := range options { 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 api.StrategyResult{ - Enabled: *opts.fallback, - } - } - return api.StrategyResult{ - Enabled: false, - } + return handleFallback(opts, feature, ctx) } if f.Dependencies != nil && len(*f.Dependencies) > 0 { @@ -292,7 +275,7 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate } } } - + if !f.Enabled { return api.StrategyResult{ Enabled: false, @@ -304,7 +287,7 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate Enabled: f.Enabled, } } - + for _, s := range f.Strategies { foundStrategy := uc.getStrategy(s.Name) if foundStrategy == nil { @@ -537,3 +520,31 @@ 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, + } +} From b2f726ba42b634e339c4d4a18ee417a7691daaf9 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 10:09:31 +0200 Subject: [PATCH 03/13] feat: add test for parent metrics --- api/feature.go | 7 +++-- client.go | 72 +++++++++++++++++++------------------------ doc.go | 16 +++++----- metrics_test.go | 77 ++++++++++++++++++++++++++++++++++++++++++++++ repository_test.go | 4 +-- utils.go | 14 ++++----- 6 files changed, 130 insertions(+), 60 deletions(-) diff --git a/api/feature.go b/api/feature.go index 7f4fff4..c0dd92c 100644 --- a/api/feature.go +++ b/api/feature.go @@ -53,11 +53,12 @@ type Feature struct { } type FeatureDependencies struct { - // The name of the feature toggle we depend upon + // Feature is the name of the feature toggle we depend upon Feature string `json:"feature"` - // Possible support for depending on a variant in the future + // 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"` } diff --git a/client.go b/client.go index 5ed410c..d4d1a71 100644 --- a/client.go +++ b/client.go @@ -242,11 +242,10 @@ func (uc *Client) IsEnabled(feature string, options ...FeatureOption) (enabled b defer func() { uc.metrics.count(feature, enabled) }() - + 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) api.StrategyResult { @@ -254,7 +253,7 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate for _, o := range options { o(&opts) } - + f := resolveToggle(uc, opts, feature) ctx := uc.staticContext @@ -274,7 +273,7 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate Enabled: false, } } - } + } if !f.Enabled { return api.StrategyResult{ @@ -287,7 +286,7 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate Enabled: f.Enabled, } } - + for _, s := range f.Strategies { foundStrategy := uc.getStrategy(s.Name) if foundStrategy == nil { @@ -341,46 +340,40 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate } 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) - for _, parent := range *feature.Dependencies { - parentToggle := uc.repository.getToggle(parent.Feature) - - if parentToggle == nil { - return false - } + if parentToggle == nil { + return false + } - if parentToggle.Dependencies != nil && len(*parentToggle.Dependencies) > 0 { - 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 - if contains(*parent.Variants, variantName) { - continue - } - } else { - if uc.isEnabled(parent.Feature, WithContext(context)).Enabled { - continue - } - } - } else { - if !uc.isEnabled(parent.Feature, WithContext(context)).Enabled { - continue - } - } - - return false - } - - return true -} + if parent.Enabled == nil { + if parent.Variants != nil && len(*parent.Variants) > 0 { + variantName := uc.getVariantWithoutMetrics(parent.Feature, WithVariantContext(context)).Name + if contains(*parent.Variants, variantName) { + continue + } + } else { + if uc.isEnabled(parent.Feature, WithContext(context)).Enabled { + continue + } + } + } else { + if !uc.isEnabled(parent.Feature, WithContext(context)).Enabled { + continue + } + } + return false + } + return true +} // GetVariant queries a variant as the specified feature is enabled. // @@ -521,7 +514,6 @@ 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 { diff --git a/doc.go b/doc.go index 2175281..a505430 100644 --- a/doc.go +++ b/doc.go @@ -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 @@ -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 diff --git a/metrics_test.go b/metrics_test.go index 0fea62a..24b329f 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -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.FeatureDependencies{ + { + 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") +} diff --git a/repository_test.go b/repository_test.go index 350825a..bc8ce2e 100644 --- a/repository_test.go +++ b/repository_test.go @@ -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 @@ -126,4 +126,4 @@ func TestRepository_ParseAPIResponse(t *testing.T) { assert.Equal(2, len(response.Features)) assert.Equal(0, len(response.Segments)) -} \ No newline at end of file +} diff --git a/utils.go b/utils.go index 0b12072..9e2eebf 100644 --- a/utils.go +++ b/utils.go @@ -36,10 +36,10 @@ func getFetchURLPath(projectName string) string { } func contains(arr []string, str string) bool { - for _, item := range arr { - if item == str { - return true - } - } - return false -} \ No newline at end of file + for _, item := range arr { + if item == str { + return true + } + } + return false +} From e01a9cd9965142d5a4d4dd328168f1444cb0a5df Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 10:41:59 +0200 Subject: [PATCH 04/13] fix: add warning --- client.go | 3 +++ utils.go | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/client.go b/client.go index d4d1a71..0b929d0 100644 --- a/client.go +++ b/client.go @@ -340,10 +340,13 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate } func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context context.Context) bool { + warnOnce := &WarnOnce{} + for _, parent := range *feature.Dependencies { 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 } diff --git a/utils.go b/utils.go index 9e2eebf..8ba2c36 100644 --- a/utils.go +++ b/utils.go @@ -5,6 +5,7 @@ import ( "math/rand" "os" "os/user" + "sync" "time" ) @@ -43,3 +44,15 @@ func contains(arr []string, str string) bool { } 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) + }) +} From ec6bd84a6e6ba0dbb5bb36ab6f425e29ea69c73c Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 10:54:23 +0200 Subject: [PATCH 05/13] feat: add custom every function --- client.go | 86 ++++++++++++++++++++++++++++++++++++++----------------- utils.go | 17 +++++++++++ 2 files changed, 77 insertions(+), 26 deletions(-) diff --git a/client.go b/client.go index 0b929d0..7c55792 100644 --- a/client.go +++ b/client.go @@ -342,40 +342,74 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context context.Context) bool { warnOnce := &WarnOnce{} - for _, parent := range *feature.Dependencies { - parentToggle := uc.repository.getToggle(parent.Feature) + dependenciesSatisfied := func(parent api.FeatureDependencies) 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 == 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 - } + 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 - if contains(*parent.Variants, variantName) { - continue - } - } else { - if uc.isEnabled(parent.Feature, WithContext(context)).Enabled { - continue + // 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 } - } else { - if !uc.isEnabled(parent.Feature, WithContext(context)).Enabled { - continue - } + + return !uc.isEnabled(parent.Feature, WithContext(context)).Enabled } - return false - } + allDependenciesSatisfied := every(*feature.Dependencies, func(parent interface{}) bool { + return dependenciesSatisfied(parent.(api.FeatureDependencies)) + }) + + if !allDependenciesSatisfied { + return false + } - return true + return true + + // for _, parent := range *feature.Dependencies { + // 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 + // if contains(*parent.Variants, variantName) { + // continue + // } + // } else { + // if uc.isEnabled(parent.Feature, WithContext(context)).Enabled { + // continue + // } + // } + // } else { + // if !uc.isEnabled(parent.Feature, WithContext(context)).Enabled { + // continue + // } + // } + + // return false + // } + + // return true } // GetVariant queries a variant as the specified feature is enabled. diff --git a/utils.go b/utils.go index 8ba2c36..ec94fbc 100644 --- a/utils.go +++ b/utils.go @@ -5,6 +5,7 @@ import ( "math/rand" "os" "os/user" + "reflect" "sync" "time" ) @@ -56,3 +57,19 @@ func (wo *WarnOnce) Warn(message string) { fmt.Println("Warning:", message) }) } + +func every(slice interface{}, condition func(interface{}) bool) bool { + sliceValue := reflect.ValueOf(slice) + + if sliceValue.Kind() != reflect.Slice { + panic("Input is not a slice") + } + + for i := 0; i < sliceValue.Len(); i++ { + element := sliceValue.Index(i).Interface() + if !condition(element) { + return false + } + } + return true +} From b80180c6ba877d215b955d8f869f684c9d2f4741 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 10:54:55 +0200 Subject: [PATCH 06/13] fix: format --- api/feature.go | 2 +- api/feature_test.go | 39 ++++++++++++++++++------------------ client.go | 48 ++++++++++++++++++++++----------------------- 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/api/feature.go b/api/feature.go index c0dd92c..a51c6c5 100644 --- a/api/feature.go +++ b/api/feature.go @@ -60,7 +60,7 @@ type FeatureDependencies struct { // 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{} { features := map[string]interface{}{} diff --git a/api/feature_test.go b/api/feature_test.go index 9f6a864..97741c0 100644 --- a/api/feature_test.go +++ b/api/feature_test.go @@ -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) } - diff --git a/client.go b/client.go index 7c55792..d918a3e 100644 --- a/client.go +++ b/client.go @@ -343,38 +343,38 @@ func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context cont warnOnce := &WarnOnce{} dependenciesSatisfied := func(parent api.FeatureDependencies) bool { - parentToggle := uc.repository.getToggle(parent.Feature) + 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 == 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 - } + 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 + // 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.FeatureDependencies)) - }) + return !uc.isEnabled(parent.Feature, WithContext(context)).Enabled + } - if !allDependenciesSatisfied { - return false - } + allDependenciesSatisfied := every(*feature.Dependencies, func(parent interface{}) bool { + return dependenciesSatisfied(parent.(api.FeatureDependencies)) + }) + + if !allDependenciesSatisfied { + return false + } - return true + return true // for _, parent := range *feature.Dependencies { // parentToggle := uc.repository.getToggle(parent.Feature) From cb2b29e847240272267a3bc97b0bbedc004a5b2c Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 11:04:30 +0200 Subject: [PATCH 07/13] fix: test for every --- utils.go | 4 ++++ utils_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/utils.go b/utils.go index ec94fbc..20d58b1 100644 --- a/utils.go +++ b/utils.go @@ -65,6 +65,10 @@ func every(slice interface{}, condition func(interface{}) bool) bool { panic("Input is not a slice") } + if (sliceValue.Len() == 0) { + return false + } + for i := 0; i < sliceValue.Len(); i++ { element := sliceValue.Index(i).Interface() if !condition(element) { diff --git a/utils_test.go b/utils_test.go index 7574e84..14e3d7e 100644 --- a/utils_test.go +++ b/utils_test.go @@ -15,3 +15,45 @@ func TestGetFetchURLPath(t *testing.T) { res = getFetchURLPath("myProject") assert.Equal("./client/features?project=myProject", res) } +func TestEvery(t *testing.T) { + // Test case 1: Check if all integers in the slice are even. + numbers1 := []int{2, 4, 6, 8, 10} + allEven := every(numbers1, func(item interface{}) bool { + num, ok := item.(int) + if !ok { + t.Errorf("Expected an integer, got %T", item) + return false + } + return num%2 == 0 + }) + if !allEven { + t.Errorf("Expected all numbers to be even, but got false") + } + + // Test case 2: Check if all strings in the slice have more than 3 characters. + words := []string{"apple", "banana", "cherry"} + allLong := every(words, func(item interface{}) bool { + str, ok := item.(string) + if !ok { + t.Errorf("Expected a string, got %T", item) + return false + } + return len(str) > 3 + }) + if !allLong { + t.Errorf("Expected all words to be long, but got false") + } + + // Test case 3: Check an empty slice. + emptySlice := []int{} + allEmpty := every(emptySlice, func(item interface{}) bool { + // This condition should not be reached for an empty slice. + t.Errorf("Unexpected condition reached") + return false + }) + + if allEmpty { + t.Errorf("Expected an empty slice to return false, but got true") + } +} + From f200237c4c232f20cf29f7f027f6a8d5c51b532f Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 11:05:35 +0200 Subject: [PATCH 08/13] fix: remove comments --- client.go | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/client.go b/client.go index d918a3e..e4a88a0 100644 --- a/client.go +++ b/client.go @@ -375,41 +375,6 @@ func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context cont } return true - - // for _, parent := range *feature.Dependencies { - // 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 - // if contains(*parent.Variants, variantName) { - // continue - // } - // } else { - // if uc.isEnabled(parent.Feature, WithContext(context)).Enabled { - // continue - // } - // } - // } else { - // if !uc.isEnabled(parent.Feature, WithContext(context)).Enabled { - // continue - // } - // } - - // return false - // } - - // return true } // GetVariant queries a variant as the specified feature is enabled. From 5a8ba9fda5df2fbadf5f120a66d81bc8395c658f Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 11:16:20 +0200 Subject: [PATCH 09/13] fix: avoid panicing --- utils.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils.go b/utils.go index 20d58b1..aac3feb 100644 --- a/utils.go +++ b/utils.go @@ -62,7 +62,8 @@ func every(slice interface{}, condition func(interface{}) bool) bool { sliceValue := reflect.ValueOf(slice) if sliceValue.Kind() != reflect.Slice { - panic("Input is not a slice") + fmt.Println("Input is not a slice returning") + return false } if (sliceValue.Len() == 0) { From e228c44fb451eff7b9ea744574cc6fb2cba0fca5 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 11:17:46 +0200 Subject: [PATCH 10/13] fix: improve error message --- utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils.go b/utils.go index aac3feb..d79d3d5 100644 --- a/utils.go +++ b/utils.go @@ -62,7 +62,7 @@ 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") + fmt.Println("Input is not a slice returning false") return false } From 0d06ff34d3df534e9f84d3ee60efabe94f0f95bc Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 15:41:38 +0200 Subject: [PATCH 11/13] fix: rename dependencies --- api/feature.go | 4 ++-- client.go | 8 ++++---- metrics_test.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/feature.go b/api/feature.go index a51c6c5..085d4bb 100644 --- a/api/feature.go +++ b/api/feature.go @@ -49,10 +49,10 @@ type Feature struct { Variants []VariantInternal `json:"variants"` // Dependencies is a list of feature toggle dependency objects - Dependencies *[]FeatureDependencies `json:"dependencies"` + Dependencies *[]Dependency `json:"dependencies"` } -type FeatureDependencies struct { +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 diff --git a/client.go b/client.go index e4a88a0..5b2bd2e 100644 --- a/client.go +++ b/client.go @@ -266,9 +266,9 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate } if f.Dependencies != nil && len(*f.Dependencies) > 0 { - parentEnabled := uc.isParentDependencySatisfied(f, *ctx) + dependenciesSatisfied := uc.isParentDependencySatisfied(f, *ctx) - if !parentEnabled { + if !dependenciesSatisfied { return api.StrategyResult{ Enabled: false, } @@ -342,7 +342,7 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.Strate func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context context.Context) bool { warnOnce := &WarnOnce{} - dependenciesSatisfied := func(parent api.FeatureDependencies) bool { + dependenciesSatisfied := func(parent api.Dependency) bool { parentToggle := uc.repository.getToggle(parent.Feature) if parentToggle == nil { @@ -367,7 +367,7 @@ func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context cont } allDependenciesSatisfied := every(*feature.Dependencies, func(parent interface{}) bool { - return dependenciesSatisfied(parent.(api.FeatureDependencies)) + return dependenciesSatisfied(parent.(api.Dependency)) }) if !allDependenciesSatisfied { diff --git a/metrics_test.go b/metrics_test.go index 24b329f..0a86830 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -307,7 +307,7 @@ func TestMetrics_ShouldNotCountMetricsForParentToggles(t *testing.T) { }, }, }, - Dependencies: &[]api.FeatureDependencies{ + Dependencies: &[]api.Dependency{ { Feature: "parent", }, From 94ece706f3f642506458c34269c8c200c61d817b Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 16:03:21 +0200 Subject: [PATCH 12/13] fix: add more test cases --- utils_test.go | 109 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 77 insertions(+), 32 deletions(-) diff --git a/utils_test.go b/utils_test.go index 14e3d7e..e70d371 100644 --- a/utils_test.go +++ b/utils_test.go @@ -15,45 +15,90 @@ func TestGetFetchURLPath(t *testing.T) { res = getFetchURLPath("myProject") assert.Equal("./client/features?project=myProject", res) } + func TestEvery(t *testing.T) { - // Test case 1: Check if all integers in the slice are even. - numbers1 := []int{2, 4, 6, 8, 10} - allEven := every(numbers1, func(item interface{}) bool { - num, ok := item.(int) - if !ok { - t.Errorf("Expected an integer, got %T", item) - return false + t.Run("All Even Integers", func(t *testing.T) { + numbers := []int{2, 4, 6, 8, 10} + allEven := every(numbers, func(item interface{}) bool { + num, ok := item.(int) + if !ok { + t.Errorf("Expected an integer, got %T", item) + return false + } + return num%2 == 0 + }) + if !allEven { + t.Errorf("Expected all numbers to be even, but got false") + } + }) + + t.Run("All Long Strings", func(t *testing.T) { + words := []string{"apple", "banana", "cherry"} + allLong := every(words, func(item interface{}) bool { + str, ok := item.(string) + if !ok { + t.Errorf("Expected a string, got %T", item) + return false + } + return len(str) > 3 + }) + if !allLong { + t.Errorf("Expected all words to be long, but got false") } - return num%2 == 0 }) - if !allEven { - t.Errorf("Expected all numbers to be even, but got false") - } - - // Test case 2: Check if all strings in the slice have more than 3 characters. - words := []string{"apple", "banana", "cherry"} - allLong := every(words, func(item interface{}) bool { - str, ok := item.(string) - if !ok { - t.Errorf("Expected a string, got %T", item) + + t.Run("Empty Slice", func(t *testing.T) { + emptySlice := []int{} + allEmpty := every(emptySlice, func(item interface{}) bool { + // This condition should not be reached for an empty slice. + t.Errorf("Unexpected condition reached") return false + }) + + if allEmpty { + t.Errorf("Expected an empty slice to return false, but got true") } - return len(str) > 3 }) - if !allLong { - t.Errorf("Expected all words to be long, but got false") - } - - // Test case 3: Check an empty slice. - emptySlice := []int{} - allEmpty := every(emptySlice, func(item interface{}) bool { - // This condition should not be reached for an empty slice. - t.Errorf("Unexpected condition reached") - return false + + t.Run("invalid inout", func(t *testing.T) { + invalidInput := "string" + result := every(invalidInput, func(item interface{}) bool { + // This condition should not be reached for an empty slice. + return true + }) + + if result == true { + t.Errorf("Expected result to be false") + } }) +} - if allEmpty { - t.Errorf("Expected an empty slice to return false, but got true") - } +func TestContains(t *testing.T) { + t.Run("Element is present in the slice", func(t *testing.T) { + arr := []string{"apple", "banana", "cherry", "date", "fig"} + str := "banana" + result := contains(arr, str) + if !result { + t.Errorf("Expected '%s' to be in the slice, but it was not found", str) + } + }) + + t.Run("Element is not present in the slice", func(t *testing.T) { + arr := []string{"apple", "banana", "cherry", "date", "fig"} + str := "grape" + result := contains(arr, str) + if result { + t.Errorf("Expected '%s' not to be in the slice, but it was found", str) + } + }) + + t.Run("Empty slice should return false", func(t *testing.T) { + arr := []string{} + str := "apple" + result := contains(arr, str) + if result { + t.Errorf("Expected an empty slice to return false, but it returned true") + } + }) } From 921b3f8c65c46b7681491d5b7343727c34690d23 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 16:04:05 +0200 Subject: [PATCH 13/13] fix: finish last test --- utils_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/utils_test.go b/utils_test.go index e70d371..204269b 100644 --- a/utils_test.go +++ b/utils_test.go @@ -71,6 +71,21 @@ func TestEvery(t *testing.T) { t.Errorf("Expected result to be false") } }) + + t.Run("Result should be false if one doesn't match the predicate", func(t *testing.T) { + words := []string{"apple", "banana", "cherry", "he"} + allLong := every(words, func(item interface{}) bool { + str, ok := item.(string) + if !ok { + t.Errorf("Expected a string, got %T", item) + return false + } + return len(str) > 3 + }) + if allLong == true { + t.Errorf("Expected all words to be long, but got false") + } + }) } func TestContains(t *testing.T) {