fix: variant fallback usage

client.go

@@ -416,29 +416,33 @@ func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOpt
 		strategyResult, f = uc.isEnabled(feature, WithContext(*ctx))
 	}
 
-	if !strategyResult.Enabled {
-		return defaultVariant
-	}
-
-	if f == nil {
+	getFallbackVariant := func(featureEnabled bool) *api.Variant {
 		if opts.variantFallbackFunc != nil {
 			return opts.variantFallbackFunc(feature, ctx)
 		} else if opts.variantFallback != nil {
 			return opts.variantFallback
 		}
+
+		if featureEnabled {
+			return disabledVariantFeatureEnabled
+		}
 		return defaultVariant
 	}
 
-	if !f.Enabled {
-		return defaultVariant
+	if !strategyResult.Enabled {
+		return getFallbackVariant(false)
+	}
+
+	if f == nil || !f.Enabled {
+		return getFallbackVariant(false)
 	}
 
 	if strategyResult.Variant != nil {
 		return strategyResult.Variant
 	}
 
 	if len(f.Variants) == 0 {
-		return disabledVariantFeatureEnabled
+		return getFallbackVariant(true)
 	}
 
 	return api.VariantCollection{

client_test.go

@@ -1144,3 +1144,276 @@ func TestClient_VariantFromEnabledFeatureWithNoVariants(t *testing.T) {
 
 	assert.True(gock.IsDone(), "there should be no more mocks")
 }
+
+func TestGetVariantWithFallbackVariantWhenFeatureDisabled(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-disabled"
+	features := []api.Feature{
+		{
+			Name:        feature,
+			Description: "feature-desc",
+			Enabled:     false,
+			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{},
+		})
+
+	client, err := NewClient(
+		WithUrl(mockerServer),
+		WithAppName(mockAppName),
+		WithInstanceId(mockInstanceId),
+		WithListener(&NoopListener{}),
+	)
+
+	assert.NoError(err)
+	client.WaitForReady()
+
+	fallbackVariant := api.Variant{
+		Name: "fallback-variant",
+	}
+
+	variant := client.GetVariant(feature, WithVariantFallback(&fallbackVariant))
+
+	assert.False(variant.Enabled)
+
+	assert.False(variant.FeatureEnabled)
+
+	assert.Equal(fallbackVariant, *variant)
+
+	fallbackFunc := func(feature string, ctx *context.Context) *api.Variant {
+		return &fallbackVariant
+	}
+
+	variantWithFallbackFunc := client.GetVariant(feature, WithVariantFallbackFunc(fallbackFunc))
+
+	assert.Equal(fallbackVariant, *variantWithFallbackFunc)
+
+	assert.True(gock.IsDone(), "there should be no more mocks")
+}
+
+func TestGetVariantWithFallbackVariantWhenFeatureEnabledButNoVariants(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{},
+		})
+
+	client, err := NewClient(
+		WithUrl(mockerServer),
+		WithAppName(mockAppName),
+		WithInstanceId(mockInstanceId),
+		WithListener(&NoopListener{}),
+	)
+
+	assert.NoError(err)
+	client.WaitForReady()
+
+	fallbackVariant := api.Variant{
+		Name: "fallback-variant",
+	}
+
+	variant := client.GetVariant(feature, WithVariantFallback(&fallbackVariant))
+
+	assert.False(variant.Enabled)
+
+	// Because we return the fallback variant and the variant
+	// doesn't have a FeatureEnabled property, it will always be
+	// false, regardless of whether the actual feature is enabled
+	// or not.
+	assert.False(variant.FeatureEnabled)
+
+	assert.Equal(fallbackVariant, *variant)
+
+	fallbackFunc := func(feature string, ctx *context.Context) *api.Variant {
+		return &fallbackVariant
+	}
+
+	variantWithFallbackFunc := client.GetVariant(feature, WithVariantFallbackFunc(fallbackFunc))
+
+	assert.Equal(fallbackVariant, *variantWithFallbackFunc)
+
+	assert.True(gock.IsDone(), "there should be no more mocks")
+}
+
+func TestGetVariantWithFallbackVariantWhenFeatureDoesntExist(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{}
+
+	gock.New(mockerServer).
+		Get("/client/features").
+		Reply(200).
+		JSON(api.FeatureResponse{
+			Features: features,
+			Segments: []api.Segment{},
+		})
+
+	client, err := NewClient(
+		WithUrl(mockerServer),
+		WithAppName(mockAppName),
+		WithInstanceId(mockInstanceId),
+		WithListener(&NoopListener{}),
+	)
+
+	assert.NoError(err)
+	client.WaitForReady()
+
+	fallbackVariant := api.Variant{
+		Name: "fallback-variant",
+	}
+
+	variant := client.GetVariant(feature, WithVariantFallback(&fallbackVariant))
+
+	assert.False(variant.Enabled)
+
+	assert.False(variant.FeatureEnabled)
+
+	assert.Equal(fallbackVariant, *variant)
+
+	fallbackFunc := func(feature string, ctx *context.Context) *api.Variant {
+		return &fallbackVariant
+	}
+
+	variantWithFallbackFunc := client.GetVariant(feature, WithVariantFallbackFunc(fallbackFunc))
+
+	assert.Equal(fallbackVariant, *variantWithFallbackFunc)
+
+	assert.True(gock.IsDone(), "there should be no more mocks")
+}
+
+func TestGetVariant_FallbackVariantFeatureEnabledSettingIsLeftUnchanged(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)
+
+	enabledFeatureNoVariants := "enabled-feature"
+	disabledFeature := "disabled-feature"
+	features := []api.Feature{
+		{
+			Name:        disabledFeature,
+			Description: "feature-desc",
+			Enabled:     false,
+			CreatedAt:   time.Date(1974, time.May, 19, 1, 2, 3, 4, time.UTC),
+			Strategy:    "default-strategy",
+			Strategies: []api.Strategy{
+				{
+					Id:   1,
+					Name: "default",
+				},
+			},
+		},
+		{
+			Name:        enabledFeatureNoVariants,
+			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{},
+		})
+
+	client, err := NewClient(
+		WithUrl(mockerServer),
+		WithAppName(mockAppName),
+		WithInstanceId(mockInstanceId),
+		WithListener(&NoopListener{}),
+	)
+
+	assert.NoError(err)
+	client.WaitForReady()
+
+	fallbackVariantFeatureEnabled := api.Variant{
+		Name: "fallback-variant-feature-enabled",
+                FeatureEnabled: true,
+	}
+	fallbackVariantFeatureDisabled := api.Variant{
+		Name: "fallback-variant-feature-disabled",
+                FeatureEnabled: false,
+	}
+
+	variantForEnabledFeatureNoVariants := client.GetVariant(enabledFeatureNoVariants, WithVariantFallback(&fallbackVariantFeatureDisabled))
+
+	assert.False(variantForEnabledFeatureNoVariants.FeatureEnabled)
+
+	variantForDisabledFeature := client.GetVariant(disabledFeature, WithVariantFallback(&fallbackVariantFeatureEnabled))
+
+	assert.True(variantForDisabledFeature.FeatureEnabled)
+
+	assert.True(gock.IsDone(), "there should be no more mocks")
+}

config.go

@@ -214,16 +214,30 @@ type variantOption struct {
 // VariantOption provides options for querying if a variant is found or not.
 type VariantOption func(*variantOption)
 
-// WithVariantFallback specifies what the value should be if the variant is not found on the
-// unleash service.
+// WithVariantFallback specifies what the value should be if the
+// variant is not found on the unleash service. This could be because
+// the feature doesn't exist, because it is disabled, or because it
+// has no variants.
+//
+// If you specify a fallback variant, note that its `FeatureEnabled`
+// field will be set to whatever you pass in or `false` by default. In
+// other words, it will not reflect the feature's actual enabled
+// state.
 func WithVariantFallback(variantFallback *api.Variant) VariantOption {
 	return func(opts *variantOption) {
 		opts.variantFallback = variantFallback
 	}
 }
 
-// WithVariantFallbackFunc specifies a fallback function to evaluate a variant
-// is not found on the service.
+// WithVariantFallbackFunc specifies a fallback function to evaluate
+// to a variant when a variant is not found for a feature. This could
+// be because the feature doesn't exist, because it is disabled, or
+// because it has no variants.
+//
+// If you specify a fallback variant, note that its `FeatureEnabled`
+// field will be set to whatever you pass in or `false` by default. In
+// other words, it will not reflect the feature's actual enabled
+// state.
 func WithVariantFallbackFunc(variantFallbackFunc VariantFallbackFunc) VariantOption {
 	return func(opts *variantOption) {
 		opts.variantFallbackFunc = variantFallbackFunc