Revert "feat: add FeatureEnabled to Variant (#164)" (#165)

This reverts commit 170cce6.

api/feature.go

@@ -89,7 +89,6 @@ 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,9 +3,8 @@ package api
 import "github.com/Unleash/unleash-client-go/v4/context"
 
 var DISABLED_VARIANT = &Variant{
-	Name:           "disabled",
-	Enabled:        false,
-	FeatureEnabled: false,
+	Name:    "disabled",
+	Enabled: false,
 }
 
 type Payload struct {
@@ -27,11 +26,8 @@ type Variant struct {
 	Name string `json:"name"`
 	// Payload is the value of the variant payload
 	Payload Payload `json:"payload"`
-	// Enabled indicates whether the variant is enabled. This is only false when
-	// it's a default variant.
+	// Enabled indicates whether the feature which is extend by this variant was enabled or not.
 	Enabled bool `json:"enabled"`
-	// FeatureEnabled indicates whether the Feature for this variant is enabled.
-	FeatureEnabled bool `json:"featureEnabled"`
 }
 
 type VariantInternal struct {
@@ -85,10 +81,7 @@ func (o Override) matchValue(ctx *context.Context) bool {
 	return false
 }
 
-// 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.
+// Get default variant if no variant is found
 func GetDefaultVariant() *Variant {
 	return DISABLED_VARIANT
 }

client.go

@@ -31,14 +31,6 @@ 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
@@ -389,7 +381,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.FeatureEnabled, variant.Name)
+		uc.metrics.countVariants(feature, variant.Enabled, variant.Name)
 	}()
 	return variant
 }
@@ -444,7 +436,7 @@ func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOpt
 	}
 
 	if len(f.Variants) == 0 {
-		return disabledVariantFeatureEnabled
+		return defaultVariant
 	}
 
 	return api.VariantCollection{

client_test.go

@@ -740,8 +740,6 @@ 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 {
@@ -755,8 +753,6 @@ func TestClient_VariantShouldRespectConstraint(t *testing.T) {
 
 	assert.True(variantFromResolver.Enabled)
 
-	assert.True(variantFromResolver.FeatureEnabled)
-
 	assert.True(gock.IsDone(), "there should be no more mocks")
 }
 
@@ -859,8 +855,6 @@ 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 {
@@ -874,8 +868,6 @@ func TestClient_VariantShouldFailWhenSegmentConstraintsDontMatch(t *testing.T) {
 
 	assert.False(variantFromResolver.Enabled)
 
-	assert.False(variantFromResolver.FeatureEnabled)
-
 	assert.True(gock.IsDone(), "there should be no more mocks")
 }
 
@@ -961,8 +953,6 @@ 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")
@@ -1061,86 +1051,7 @@ 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,17 +48,14 @@ func Test_withVariants(t *testing.T) {
 		t.Fail()
 	}
 
-	variant := unleash.GetVariant("Demo")
-	if variant.Enabled == false {
-		t.Fatalf("Expected variant to be enabled")
-	}
-	if variant.FeatureEnabled == false {
+	feature := unleash.GetVariant("Demo")
+	if feature.Enabled == false {
 		t.Fatalf("Expected feature to be enabled")
 	}
-	if variant.Name != "small" && variant.Name != "medium" {
+	if feature.Name != "small" && feature.Name != "medium" {
 		t.Fatalf("Expected one of the variant names")
 	}
-	if variant.Payload.Value != "35" && variant.Payload.Value != "55" {
+	if feature.Payload.Value != "35" && feature.Payload.Value != "55" {
 		t.Fatalf("Expected one of the variant payloads")
 	}
 	err = unleash.Close()