feat: add FeatureEnabled to Variant

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 #159

[1] https://github.com/Unleash/client-specification/blob/c0169d7ace35db66cdf41a7b1b4e390a4a843c3b/specifications/08-variants.json#L448

api/feature.go

@@ -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

api/variant.go

@@ -3,8 +3,9 @@ package api
 import "github.com/Unleash/unleash-client-go/v4/context"
 
 var DISABLED_VARIANT = &Variant{
-	Name:    "disabled",
-	Enabled: false,
+	Name:           "disabled",
+	Enabled:        false,
+	FeatureEnabled: false,
 }
 
 type Payload struct {
@@ -26,8 +27,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 {
@@ -81,7 +85,10 @@ 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.
+//
+// Rather than checking against this particular variant you should be checking
+// the returned variant's Enabled and FeatureEnabled properties.
 func GetDefaultVariant() *Variant {
 	return DISABLED_VARIANT
 }

client.go

@@ -31,6 +31,14 @@ var defaultStrategies = []strategy.Strategy{
 	*s.NewFlexibleRolloutStrategy(),
 }
 
+// disabledVariantFeatureEnabled is similar to api.DISABLED_VARIANT but we want
+// to discourage public usage so it's internal until there's a need to expose it.
+var disabledVariantFeatureEnabled = &api.Variant{
+	Name:           "disabled",
+	Enabled:        false,
+	FeatureEnabled: true,
+}
+
 // Client is a structure representing an API client of an Unleash server.
 type Client struct {
 	errorChannels
@@ -381,7 +389,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
 }
@@ -436,7 +444,7 @@ func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOpt
 	}
 
 	if len(f.Variants) == 0 {
-		return defaultVariant
+		return disabledVariantFeatureEnabled
 	}
 
 	return api.VariantCollection{

client_test.go

@@ -740,6 +740,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 {
@@ -753,6 +755,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")
 }
 
@@ -855,6 +859,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 {
@@ -868,6 +874,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")
 }
 
@@ -953,6 +961,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")
@@ -1051,7 +1061,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(disabledVariantFeatureEnabled, 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(disabledVariantFeatureEnabled, variantFromResolver)
+
+	assert.True(gock.IsDone(), "there should be no more mocks")
+}

unleash_test.go

@@ -48,14 +48,17 @@ func Test_withVariants(t *testing.T) {
 		t.Fail()
 	}
 
-	feature := unleash.GetVariant("Demo")
-	if feature.Enabled == false {
+	variant := unleash.GetVariant("Demo")
+	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")
 	}
 	err = unleash.Close()