Skip to content

Commit

Permalink
feat: add FeatureEnabled to Variant
Browse files Browse the repository at this point in the history
FeatureEnabled is similar to Enabled except in the case where the feature
is enabled but there are no variants defined. This follows the client
specification case [1].

Fixes Unleash#159

[1] https://github.com/Unleash/client-specification/blob/c0169d7ace35db66cdf41a7b1b4e390a4a843c3b/specifications/08-variants.json#L448
  • Loading branch information
jameshartig committed Dec 8, 2023
1 parent 6e71cb0 commit fdcbc02
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 13 deletions.
1 change: 1 addition & 0 deletions api/feature.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func (vc VariantCollection) GetVariant(ctx *context.Context) *Variant {
variant = &v.Variant
}
variant.Enabled = true
variant.FeatureEnabled = true
return variant
}
return DISABLED_VARIANT
Expand Down
28 changes: 22 additions & 6 deletions api/variant.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@ package api

import "github.com/Unleash/unleash-client-go/v4/context"

var DISABLED_VARIANT = &Variant{
Name: "disabled",
Enabled: false,
}
var (
DISABLED_VARIANT = &Variant{
Name: "disabled",
Enabled: false,
FeatureEnabled: false,
}
DISABLED_VARIANT_FEATURE_ENABLED = &Variant{
Name: "disabled",
Enabled: false,
FeatureEnabled: true,
}
)

type Payload struct {
// Type is the type of the payload
Expand All @@ -26,8 +34,11 @@ type Variant struct {
Name string `json:"name"`
// Payload is the value of the variant payload
Payload Payload `json:"payload"`
// Enabled indicates whether the feature which is extend by this variant was enabled or not.
// Enabled indicates whether the variant is enabled. This is only false when
// it's a default variant.
Enabled bool `json:"enabled"`
// FeatureEnabled indicates whether the Feature for this variant is enabled.
FeatureEnabled bool `json:"featureEnabled"`
}

type VariantInternal struct {
Expand Down Expand Up @@ -81,7 +92,12 @@ func (o Override) matchValue(ctx *context.Context) bool {
return false
}

// Get default variant if no variant is found
// Get default variant if feature is not found or if the feature is disabled.
func GetDefaultVariant() *Variant {
return DISABLED_VARIANT
}

// Get default variant if feature is enabled but no variants exist.
func GetDefaultVariantFeatureEnabled() *Variant {
return DISABLED_VARIANT_FEATURE_ENABLED
}
4 changes: 2 additions & 2 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ func (uc *Client) isParentDependencySatisfied(feature *api.Feature, context cont
func (uc *Client) GetVariant(feature string, options ...VariantOption) *api.Variant {
variant := uc.getVariantWithoutMetrics(feature, options...)
defer func() {
uc.metrics.countVariants(feature, variant.Enabled, variant.Name)
uc.metrics.countVariants(feature, variant.FeatureEnabled, variant.Name)
}()
return variant
}
Expand Down Expand Up @@ -440,7 +440,7 @@ func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOpt
}

if len(f.Variants) == 0 {
return defaultVariant
return api.GetDefaultVariantFeatureEnabled()
}

return api.VariantCollection{
Expand Down
89 changes: 89 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,8 @@ func TestClient_VariantShouldRespectConstraint(t *testing.T) {

assert.True(variant.Enabled)

assert.True(variant.FeatureEnabled)

variantFromResolver := client.GetVariant(feature, WithVariantContext(context.Context{
Properties: map[string]string{"custom-id": "custom-ctx", "semver": "3.2.2", "age": "18", "domain": "unleashtest"},
}), WithVariantResolver(func(featureName string) *api.Feature {
Expand All @@ -773,6 +775,8 @@ func TestClient_VariantShouldRespectConstraint(t *testing.T) {

assert.True(variantFromResolver.Enabled)

assert.True(variantFromResolver.FeatureEnabled)

assert.True(gock.IsDone(), "there should be no more mocks")
}

Expand Down Expand Up @@ -877,6 +881,8 @@ func TestClient_VariantShouldFailWhenSegmentConstraintsDontMatch(t *testing.T) {

assert.False(variant.Enabled)

assert.False(variant.FeatureEnabled)

variantFromResolver := client.GetVariant(feature, WithVariantContext(context.Context{
Properties: map[string]string{"custom-id": "custom-ctx", "semver": "3.2.2", "age": "18", "domain": "unleashtest"},
}), WithVariantResolver(func(featureName string) *api.Feature {
Expand All @@ -890,6 +896,8 @@ func TestClient_VariantShouldFailWhenSegmentConstraintsDontMatch(t *testing.T) {

assert.False(variantFromResolver.Enabled)

assert.False(variantFromResolver.FeatureEnabled)

assert.True(gock.IsDone(), "there should be no more mocks")
}

Expand Down Expand Up @@ -975,6 +983,8 @@ func TestClient_ShouldFavorStrategyVariantOverFeatureVariant(t *testing.T) {

assert.True(strategyVariant.Enabled)

assert.True(strategyVariant.FeatureEnabled)

assert.Equal("strategyVariantName", strategyVariant.Name)

assert.True(gock.IsDone(), "there should be no more mocks")
Expand Down Expand Up @@ -1073,7 +1083,86 @@ func TestClient_ShouldReturnOldVariantForNonMatchingStrategyVariant(t *testing.T

assert.True(strategyVariant.Enabled)

assert.True(strategyVariant.FeatureEnabled)

assert.Equal("willBeSelected", strategyVariant.Name)

assert.True(gock.IsDone(), "there should be no more mocks")
}

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

gock.New(mockerServer).
Post("/client/register").
MatchHeader("UNLEASH-APPNAME", mockAppName).
MatchHeader("UNLEASH-INSTANCEID", mockInstanceId).
Reply(200)

feature := "feature-no-variants"
features := []api.Feature{
{
Name: feature,
Description: "feature-desc",
Enabled: true,
CreatedAt: time.Date(1974, time.May, 19, 1, 2, 3, 4, time.UTC),
Strategy: "default-strategy",
Strategies: []api.Strategy{
{
Id: 1,
Name: "default",
},
},
},
}

gock.New(mockerServer).
Get("/client/features").
Reply(200).
JSON(api.FeatureResponse{
Features: features,
Segments: []api.Segment{},
})

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

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

assert.NoError(err)
client.WaitForReady()

variant := client.GetVariant(feature, WithVariantContext(context.Context{}))

assert.False(variant.Enabled)

assert.True(variant.FeatureEnabled)

assert.Equal(api.GetDefaultVariantFeatureEnabled(), variant)

variantFromResolver := client.GetVariant(feature, WithVariantContext(context.Context{}), WithVariantResolver(func(featureName string) *api.Feature {
if featureName == features[0].Name {
return &features[0]
} else {
t.Fatalf("the feature name passed %s was not the expected one %s", featureName, features[0].Name)
return nil
}
}))

assert.False(variantFromResolver.Enabled)

assert.True(variantFromResolver.FeatureEnabled)

assert.Equal(api.GetDefaultVariantFeatureEnabled(), variantFromResolver)

assert.True(gock.IsDone(), "there should be no more mocks")
}
13 changes: 8 additions & 5 deletions unleash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,18 @@ func Test_withVariants(t *testing.T) {
t.Fail()
}

feature := unleash.GetVariant("Demo")
fmt.Printf("feature is %v\n", feature)
if feature.Enabled == false {
variant := unleash.GetVariant("Demo")
fmt.Printf("variant is %v\n", variant)
if variant.Enabled == false {
t.Fatalf("Expected variant to be enabled")
}
if variant.FeatureEnabled == false {
t.Fatalf("Expected feature to be enabled")
}
if feature.Name != "small" && feature.Name != "medium" {
if variant.Name != "small" && variant.Name != "medium" {
t.Fatalf("Expected one of the variant names")
}
if feature.Payload.Value != "35" && feature.Payload.Value != "55" {
if variant.Payload.Value != "35" && variant.Payload.Value != "55" {
t.Fatalf("Expected one of the variant payloads")
}
}
Expand Down

0 comments on commit fdcbc02

Please sign in to comment.