Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add variant strategy support #135

Merged
merged 9 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
name: Checkout client specifications
with:
repository: Unleash/client-specification
ref: refs/tags/v4.1.0
ref: refs/tags/v4.3.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update client spec to latest

path: testdata/client-specification
- uses: actions/setup-go@v2
name: Setup go
Expand Down
16 changes: 8 additions & 8 deletions api/feature.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ type FeatureResponse struct {
}

type Segment struct {
Id int `json:"id"`
Constraints []Constraint `json:"constraints"`
Id int `json:"id"`
Constraints []Constraint `json:"constraints"`
}

type Feature struct {
Expand Down Expand Up @@ -63,12 +63,12 @@ func (fr FeatureResponse) SegmentsMap() map[int][]Constraint {
segments[segment.Id] = segment.Constraints
}

return segments;
return segments
}

// Get variant for a given feature which is considered as enabled
func (f Feature) GetVariant(ctx *context.Context) *Variant {
if f.Enabled && len(f.Variants) > 0 {
func (f VariantCollection) GetVariant(ctx *context.Context) *Variant {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of feature, now variant can be resolved from Strategy.

if len(f.Variants) > 0 {
v := f.getOverrideVariant(ctx)
var variant *Variant
if v == nil {
Expand All @@ -82,7 +82,7 @@ func (f Feature) GetVariant(ctx *context.Context) *Variant {
return DISABLED_VARIANT
}

func (f Feature) getVariantFromWeights(ctx *context.Context) *Variant {
func (f VariantCollection) getVariantFromWeights(ctx *context.Context) *Variant {
totalWeight := 0
for _, variant := range f.Variants {
totalWeight += variant.Weight
Expand All @@ -92,7 +92,7 @@ func (f Feature) getVariantFromWeights(ctx *context.Context) *Variant {
}
stickiness := f.Variants[0].Stickiness

target := getNormalizedNumber(getSeed(ctx, stickiness), f.Name, totalWeight)
target := getNormalizedNumber(getSeed(ctx, stickiness), f.GroupId, totalWeight)
counter := uint32(0)
for _, variant := range f.Variants {
counter += uint32(variant.Weight)
Expand All @@ -104,7 +104,7 @@ func (f Feature) getVariantFromWeights(ctx *context.Context) *Variant {
return DISABLED_VARIANT
}

func (f Feature) getOverrideVariant(ctx *context.Context) *VariantInternal {
func (f VariantCollection) getOverrideVariant(ctx *context.Context) *VariantInternal {
for _, variant := range f.Variants {
for _, override := range variant.Overrides {
if override.matchValue(ctx) {
Expand Down
8 changes: 8 additions & 0 deletions api/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ type Strategy struct {
Parameters ParameterMap `json:"parameters"`

Segments []int `json:"segments"`

// Variants for a strategy
Variants []VariantInternal `json:"variants"`
}

type ParameterDescription struct {
Expand All @@ -33,3 +36,8 @@ type StrategyDescription struct {
Description string `json:"description"`
Parameters []ParameterDescription `json:"parameters"`
}

type StrategyResult struct {
Enabled bool
Variant *Variant
}
7 changes: 7 additions & 0 deletions api/variant.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ type VariantInternal struct {
Overrides []Override `json:"overrides"`
}

type VariantCollection struct {
// groupId to evaluate the variant
GroupId string
// variants for a feature toggle or feature strategy
Variants []VariantInternal
}

func (o Override) getIdentifier(ctx *context.Context) string {
var value string
switch o.ContextName {
Expand Down
83 changes: 51 additions & 32 deletions api/variant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,12 @@ func (suite *VariantTestSuite) TestGetVariantWhenFeatureHasNoVariant() {
Enabled: true,
}
mockContext := &context.Context{}
suite.Equal(DISABLED_VARIANT, mockFeature.GetVariant(mockContext), "Should return default variant")
}
variantSetup := VariantCollection{
GroupId: mockFeature.Name,
Variants: mockFeature.Variants,
}.GetVariant(mockContext)

func (suite *VariantTestSuite) TestGetVariantWhenFeatureIsNotEnabled() {
mockFeature := Feature{
Name: "test.variants",
Enabled: false,
Variants: suite.VariantWithOverride,
}
mockContext := &context.Context{
UserId: "1",
SessionId: "ABCDE",
RemoteAddress: "127.0.0.1",
}
suite.Equal(DISABLED_VARIANT, mockFeature.GetVariant(mockContext), "Should return default variant")
suite.Equal(DISABLED_VARIANT, variantSetup, "Should return default variant")
}

func (suite *VariantTestSuite) TestGetVariant_OverrideOnUserId() {
Expand All @@ -137,9 +128,13 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnUserId() {
Type: "string",
Value: "Test 1",
}
suite.Equal("VarA", mockFeature.GetVariant(mockContext).Name, "Should return VarA")
suite.Equal(true, mockFeature.GetVariant(mockContext).Enabled, "Should be equal")
suite.Equal(expectedPayload, mockFeature.GetVariant(mockContext).Payload, "Should be equal")
variantSetup := VariantCollection{
GroupId: mockFeature.Name,
Variants: mockFeature.Variants,
}.GetVariant(mockContext)
suite.Equal("VarA", variantSetup.Name, "Should return VarA")
suite.Equal(true, variantSetup.Enabled, "Should be equal")
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
}

func (suite *VariantTestSuite) TestGetVariant_OverrideOnRemoteAddress() {
Expand All @@ -156,9 +151,13 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnRemoteAddress() {
Type: "string",
Value: "Test 2",
}
suite.Equal("VarB", mockFeature.GetVariant(mockContext).Name, "Should return VarB")
suite.Equal(true, mockFeature.GetVariant(mockContext).Enabled, "Should be equal")
suite.Equal(expectedPayload, mockFeature.GetVariant(mockContext).Payload, "Should be equal")
variantSetup := VariantCollection{
GroupId: mockFeature.Name,
Variants: mockFeature.Variants,
}.GetVariant(mockContext)
suite.Equal("VarB", variantSetup.Name, "Should return VarB")
suite.Equal(true, variantSetup.Enabled, "Should be equal")
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
}

func (suite *VariantTestSuite) TestGetVariant_OverrideOnSessionId() {
Expand All @@ -176,9 +175,13 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnSessionId() {
Type: "string",
Value: "Test 1",
}
suite.Equal("VarA", mockFeature.GetVariant(mockContext).Name, "Should return VarA")
suite.Equal(true, mockFeature.GetVariant(mockContext).Enabled, "Should be equal")
suite.Equal(expectedPayload, mockFeature.GetVariant(mockContext).Payload, "Should be equal")
variantSetup := VariantCollection{
GroupId: mockFeature.Name,
Variants: mockFeature.Variants,
}.GetVariant(mockContext)
suite.Equal("VarA", variantSetup.Name, "Should return VarA")
suite.Equal(true, variantSetup.Enabled, "Should be equal")
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
}

func (suite *VariantTestSuite) TestGetVariant_OverrideOnCustomProperties() {
Expand All @@ -196,9 +199,13 @@ func (suite *VariantTestSuite) TestGetVariant_OverrideOnCustomProperties() {
Type: "string",
Value: "Test 3",
}
suite.Equal("VarC", mockFeature.GetVariant(mockContext).Name, "Should return VarC")
suite.Equal(true, mockFeature.GetVariant(mockContext).Enabled, "Should be equal")
suite.Equal(expectedPayload, mockFeature.GetVariant(mockContext).Payload, "Should be equal")
variantSetup := VariantCollection{
GroupId: mockFeature.Name,
Variants: mockFeature.Variants,
}.GetVariant(mockContext)
suite.Equal("VarC", variantSetup.Name, "Should return VarC")
suite.Equal(true, variantSetup.Enabled, "Should be equal")
suite.Equal(expectedPayload, variantSetup.Payload, "Should be equal")
}

func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarD() {
Expand All @@ -210,8 +217,12 @@ func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarD() {
mockContext := &context.Context{
UserId: "123",
}
suite.Equal("VarD", mockFeature.GetVariant(mockContext).Name, "Should return VarD")
suite.Equal(true, mockFeature.GetVariant(mockContext).Enabled, "Should be equal")
variantSetup := VariantCollection{
GroupId: mockFeature.Name,
Variants: mockFeature.Variants,
}.GetVariant(mockContext)
suite.Equal("VarD", variantSetup.Name, "Should return VarD")
suite.Equal(true, variantSetup.Enabled, "Should be equal")
}

func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarE() {
Expand All @@ -223,8 +234,12 @@ func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarE() {
mockContext := &context.Context{
UserId: "163",
}
suite.Equal("VarE", mockFeature.GetVariant(mockContext).Name, "Should return VarE")
suite.Equal(true, mockFeature.GetVariant(mockContext).Enabled, "Should be equal")
variantSetup := VariantCollection{
GroupId: mockFeature.Name,
Variants: mockFeature.Variants,
}.GetVariant(mockContext)
suite.Equal("VarE", variantSetup.Name, "Should return VarE")
suite.Equal(true, variantSetup.Enabled, "Should be equal")
}

func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarF() {
Expand All @@ -236,8 +251,12 @@ func (suite *VariantTestSuite) TestGetVariant_ShouldReturnVarF() {
mockContext := &context.Context{
UserId: "40",
}
suite.Equal("VarF", mockFeature.GetVariant(mockContext).Name, "Should return VarF")
suite.Equal(true, mockFeature.GetVariant(mockContext).Enabled, "Should be equal")
variantSetup := VariantCollection{
GroupId: mockFeature.Name,
Variants: mockFeature.Variants,
}.GetVariant(mockContext)
suite.Equal("VarF", variantSetup.Name, "Should return VarF")
suite.Equal(true, variantSetup.Enabled, "Should be equal")
}

func TestVariantSuite(t *testing.T) {
Expand Down
69 changes: 52 additions & 17 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,12 @@ func (uc *Client) IsEnabled(feature string, options ...FeatureOption) (enabled b
uc.metrics.count(feature, enabled)
}()

return uc.isEnabled(feature, options...)
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) (enabled bool) {
func (uc *Client) isEnabled(feature string, options ...FeatureOption) api.StrategyResult {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of simple boolean, we are now returning complex object, that includes variant.

var opts featureOption
for _, o := range options {
o(&opts)
Expand All @@ -268,19 +268,29 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) (enabled b

if f == nil {
if opts.fallbackFunc != nil {
return opts.fallbackFunc(feature, ctx)
return api.StrategyResult{
Enabled: opts.fallbackFunc(feature, ctx),
}
} else if opts.fallback != nil {
return *opts.fallback
return api.StrategyResult{
Enabled: *opts.fallback,
}
}
return api.StrategyResult{
Enabled: false,
}
return false
}

if !f.Enabled {
return false
return api.StrategyResult{
Enabled: false,
}
}

if len(f.Strategies) == 0 {
return f.Enabled
return api.StrategyResult{
Enabled: f.Enabled,
}
}

for _, s := range f.Strategies {
Expand All @@ -294,7 +304,9 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) (enabled b

if err != nil {
uc.errors <- err
return false
return api.StrategyResult{
Enabled: false,
}
}

allConstraints := make([]api.Constraint, 0)
Expand All @@ -304,11 +316,25 @@ func (uc *Client) isEnabled(feature string, options ...FeatureOption) (enabled b
if ok, err := constraints.Check(ctx, allConstraints); err != nil {
uc.errors <- err
} else if ok && foundStrategy.IsEnabled(s.Parameters, ctx) {
return true
if s.Variants != nil && len(s.Variants) > 0 {
return api.StrategyResult{
Enabled: true,
Variant: api.VariantCollection{
GroupId: f.Name,
Variants: s.Variants,
}.GetVariant(ctx),
}
} else {
return api.StrategyResult{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If strategy does not have variants, just return true and use old variants.

Enabled: true,
}
}
}
}

return false
return api.StrategyResult{
Enabled: false,
}
}

// GetVariant queries a variant as the specified feature is enabled.
Expand All @@ -335,14 +361,16 @@ func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOpt
ctx = ctx.Override(*opts.ctx)
}

var strategyResult api.StrategyResult

if opts.resolver != nil {
if !uc.isEnabled(feature, WithContext(*ctx), WithResolver(opts.resolver)) {
return defaultVariant
}
strategyResult = uc.isEnabled(feature, WithContext(*ctx), WithResolver(opts.resolver))
} else {
if !uc.isEnabled(feature, WithContext(*ctx)) {
return defaultVariant
}
strategyResult = uc.isEnabled(feature, WithContext(*ctx))
}

if !strategyResult.Enabled {
return defaultVariant
}

var f *api.Feature
Expand All @@ -365,11 +393,18 @@ func (uc *Client) getVariantWithoutMetrics(feature string, options ...VariantOpt
return defaultVariant
}

if strategyResult.Variant != nil {
return strategyResult.Variant
}

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

return f.GetVariant(ctx)
return api.VariantCollection{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If code runs here, then strategy variants were not found, use old variants

GroupId: f.Name,
Variants: f.Variants,
}.GetVariant(ctx)
}

// Close stops the client from syncing data from the server.
Expand Down
Loading
Loading