From b2f726ba42b634e339c4d4a18ee417a7691daaf9 Mon Sep 17 00:00:00 2001 From: Fredrik Oseberg Date: Wed, 27 Sep 2023 10:09:31 +0200 Subject: [PATCH] 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 +}