diff --git a/cmd/editor/main.go b/cmd/editor/main.go index ec305929607..c9c06a05d4d 100644 --- a/cmd/editor/main.go +++ b/cmd/editor/main.go @@ -13,7 +13,6 @@ import ( "github.com/thomaspoignant/go-feature-flag/internal/utils" "github.com/thomaspoignant/go-feature-flag/model" "github.com/thomaspoignant/go-feature-flag/model/dto" - "github.com/thomaspoignant/go-feature-flag/utils/fflog" ) // This service is an API used to evaluate a flag with an evaluation context @@ -55,8 +54,7 @@ func EvaluateHandler(c echo.Context) error { if err := c.Bind(u); err != nil { return err } - logger := fflog.FFLogger{} - f := u.Flag.Convert(&logger, u.FlagName) + f := u.Flag.Convert() value, resolutionDetails := f.Value( u.FlagName, utils.ConvertEvaluationCtxFromRequest(u.Context.Key, u.Context.Custom), diff --git a/cmd/lint/linter.go b/cmd/lint/linter.go index 1cfb6649de3..99643e1cbd2 100644 --- a/cmd/lint/linter.go +++ b/cmd/lint/linter.go @@ -3,13 +3,11 @@ package main import ( "encoding/json" "fmt" - "log/slog" "os" "strings" "github.com/BurntSushi/toml" "github.com/thomaspoignant/go-feature-flag/model/dto" - "github.com/thomaspoignant/go-feature-flag/utils/fflog" "k8s.io/apimachinery/pkg/util/yaml" ) @@ -41,10 +39,7 @@ func (l *Linter) Lint() []error { errs := make([]error, 0) for key, flagDto := range flags { - logger := fflog.FFLogger{ - LeveledLogger: slog.Default(), - } - flag := flagDto.Convert(&logger, key) + flag := flagDto.Convert() if err := flag.IsValid(); err != nil { errs = append(errs, fmt.Errorf("%s: invalid flag %s: %w", l.InputFile, key, err)) } diff --git a/feature_flag_test.go b/feature_flag_test.go index ede71d55589..1b8d2b95469 100644 --- a/feature_flag_test.go +++ b/feature_flag_test.go @@ -128,20 +128,6 @@ func TestValidUseCase(t *testing.T) { assert.True(t, ffclient.ForceRefresh()) } -func TestAllFlagsFromCache(t *testing.T) { - err := ffclient.Init(ffclient.Config{ - Retriever: &fileretriever.Retriever{Path: "testdata/flag-config.yaml"}, - PollingInterval: 5 * time.Second, - }) - defer ffclient.Close() - - assert.NoError(t, err) - flags, err := ffclient.GetFlagsFromCache() - - assert.NoError(t, err) - assert.Len(t, flags, 2) -} - func TestValidUseCaseToml(t *testing.T) { // Valid use case gffClient, err := ffclient.New(ffclient.Config{ diff --git a/internal/cache/in_memory_cache.go b/internal/cache/in_memory_cache.go index 09bab2e3377..ad34beafa02 100644 --- a/internal/cache/in_memory_cache.go +++ b/internal/cache/in_memory_cache.go @@ -66,7 +66,7 @@ func (fc *InMemoryCache) All() map[string]flag.Flag { func (fc *InMemoryCache) Init(flags map[string]dto.DTO) { cache := make(map[string]flag.InternalFlag) for key, flagDto := range flags { - flagToAdd := flagDto.Convert(fc.Logger, key) + flagToAdd := flagDto.Convert() if err := flagToAdd.IsValid(); err == nil { cache[key] = flagToAdd } else { diff --git a/internal/cache/in_memory_cache_test.go b/internal/cache/in_memory_cache_test.go index b8f7154fd58..b3559822f16 100644 --- a/internal/cache/in_memory_cache_test.go +++ b/internal/cache/in_memory_cache_test.go @@ -20,17 +20,15 @@ func TestAll(t *testing.T) { name: "all with 1 flag", param: map[string]dto.DTO{ "test": { - DTOv1: dto.DTOv1{ - Variations: &map[string]*interface{}{ - "True": testconvert.Interface("true"), - "False": testconvert.Interface("false"), - "Default": testconvert.Interface("default"), - }, - DefaultRule: &flag.Rule{ - Percentages: &map[string]float64{ - "True": 40, - "False": 60, - }, + Variations: &map[string]*interface{}{ + "True": testconvert.Interface("true"), + "False": testconvert.Interface("false"), + "Default": testconvert.Interface("default"), + }, + DefaultRule: &flag.Rule{ + Percentages: &map[string]float64{ + "True": 40, + "False": 60, }, }, }, @@ -55,34 +53,30 @@ func TestAll(t *testing.T) { name: "all with multiple flags", param: map[string]dto.DTO{ "test": { - DTOv1: dto.DTOv1{ - Variations: &map[string]*interface{}{ - "True": testconvert.Interface("true"), - "False": testconvert.Interface("false"), - "Default": testconvert.Interface("default"), - }, - DefaultRule: &flag.Rule{ - Name: testconvert.String("defaultRule"), - Percentages: &map[string]float64{ - "True": 40, - "False": 60, - }, + Variations: &map[string]*interface{}{ + "True": testconvert.Interface("true"), + "False": testconvert.Interface("false"), + "Default": testconvert.Interface("default"), + }, + DefaultRule: &flag.Rule{ + Name: testconvert.String("defaultRule"), + Percentages: &map[string]float64{ + "True": 40, + "False": 60, }, }, }, "test1": { - DTOv1: dto.DTOv1{ - Variations: &map[string]*interface{}{ - "True": testconvert.Interface(true), - "False": testconvert.Interface(false), - "Default": testconvert.Interface(false), - }, - DefaultRule: &flag.Rule{ - Name: testconvert.String("defaultRule"), - Percentages: &map[string]float64{ - "True": 30, - "False": 70, - }, + Variations: &map[string]*interface{}{ + "True": testconvert.Interface(true), + "False": testconvert.Interface(false), + "Default": testconvert.Interface(false), + }, + DefaultRule: &flag.Rule{ + Name: testconvert.String("defaultRule"), + Percentages: &map[string]float64{ + "True": 30, + "False": 70, }, }, }, @@ -142,18 +136,16 @@ func TestCopy(t *testing.T) { name: "copy with 1 flag", param: map[string]dto.DTO{ "test": { - DTOv1: dto.DTOv1{ - Variations: &map[string]*interface{}{ - "True": testconvert.Interface("true"), - "False": testconvert.Interface("false"), - "Default": testconvert.Interface("default"), - }, - DefaultRule: &flag.Rule{ - Name: testconvert.String("defaultRule"), - Percentages: &map[string]float64{ - "True": 40, - "False": 60, - }, + Variations: &map[string]*interface{}{ + "True": testconvert.Interface("true"), + "False": testconvert.Interface("false"), + "Default": testconvert.Interface("default"), + }, + DefaultRule: &flag.Rule{ + Name: testconvert.String("defaultRule"), + Percentages: &map[string]float64{ + "True": 40, + "False": 60, }, }, }, diff --git a/model/dto/convert_v1.go b/model/dto/converter.go similarity index 75% rename from model/dto/convert_v1.go rename to model/dto/converter.go index 65823d7afd9..3e8ab12e37b 100644 --- a/model/dto/convert_v1.go +++ b/model/dto/converter.go @@ -1,9 +1,11 @@ package dto -import "github.com/thomaspoignant/go-feature-flag/internal/flag" +import ( + "github.com/thomaspoignant/go-feature-flag/internal/flag" +) -// ConvertV1DtoToInternalFlag is converting a DTO to a flag.InternalFlag -func ConvertV1DtoToInternalFlag(dto DTO) flag.InternalFlag { +// ConvertDtoToInternalFlag is converting a DTO to a flag.InternalFlag +func ConvertDtoToInternalFlag(dto DTO) flag.InternalFlag { var experimentation *flag.ExperimentationRollout if dto.Experimentation != nil { experimentation = &flag.ExperimentationRollout{ diff --git a/model/dto/converter_test.go b/model/dto/converter_test.go index 7bd44013ffa..290c6ea9b77 100644 --- a/model/dto/converter_test.go +++ b/model/dto/converter_test.go @@ -19,57 +19,55 @@ func TestConvertV1DtoToInternalFlag(t *testing.T) { { name: "all fields set", input: dto.DTO{ - DTOv1: dto.DTOv1{ - BucketingKey: testconvert.String("bucketKey"), - Variations: &map[string]*interface{}{ - "var1": testconvert.Interface("var1"), - "var2": testconvert.Interface("var2"), - }, - Rules: &[]flag.Rule{{ - Name: testconvert.String("rule1"), - Query: testconvert.String("key eq \"key\""), - Percentages: &map[string]float64{"var_true": 100, "var_false": 0}}, - { - Name: testconvert.String("rule2"), - Query: testconvert.String("key eq \"key2\""), - ProgressiveRollout: &flag.ProgressiveRollout{ - Initial: &flag.ProgressiveRolloutStep{ - Variation: testconvert.String("var1"), - Percentage: testconvert.Float64(30), - Date: testconvert.Time(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)), - }, - End: &flag.ProgressiveRolloutStep{ - Variation: testconvert.String("var2"), - Percentage: testconvert.Float64(70), - Date: testconvert.Time(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)), - }, + BucketingKey: testconvert.String("bucketKey"), + Variations: &map[string]*interface{}{ + "var1": testconvert.Interface("var1"), + "var2": testconvert.Interface("var2"), + }, + Rules: &[]flag.Rule{{ + Name: testconvert.String("rule1"), + Query: testconvert.String("key eq \"key\""), + Percentages: &map[string]float64{"var_true": 100, "var_false": 0}}, + { + Name: testconvert.String("rule2"), + Query: testconvert.String("key eq \"key2\""), + ProgressiveRollout: &flag.ProgressiveRollout{ + Initial: &flag.ProgressiveRolloutStep{ + Variation: testconvert.String("var1"), + Percentage: testconvert.Float64(30), + Date: testconvert.Time(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)), + }, + End: &flag.ProgressiveRolloutStep{ + Variation: testconvert.String("var2"), + Percentage: testconvert.Float64(70), + Date: testconvert.Time(time.Date(2022, 1, 1, 0, 0, 0, 0, time.UTC)), }, - }, - { - Name: testconvert.String("rule3"), - Query: testconvert.String("key eq \"key3\""), - VariationResult: testconvert.String("var2"), }, }, - DefaultRule: &flag.Rule{ - VariationResult: testconvert.String("var1"), + { + Name: testconvert.String("rule3"), + Query: testconvert.String("key eq \"key3\""), + VariationResult: testconvert.String("var2"), }, - Scheduled: &[]flag.ScheduledStep{ - { - Date: testconvert.Time(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)), - InternalFlag: flag.InternalFlag{ - DefaultRule: &flag.Rule{ - VariationResult: testconvert.String("var2"), - }, + }, + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("var1"), + }, + Scheduled: &[]flag.ScheduledStep{ + { + Date: testconvert.Time(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)), + InternalFlag: flag.InternalFlag{ + DefaultRule: &flag.Rule{ + VariationResult: testconvert.String("var2"), }, }, }, - Experimentation: &dto.ExperimentationDto{ - Start: testconvert.Time(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)), - End: testconvert.Time(time.Date(2022, 1, 2, 0, 0, 0, 0, time.UTC)), - }, - Metadata: &map[string]interface{}{"key": "value"}, }, + Experimentation: &dto.ExperimentationDto{ + Start: testconvert.Time(time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)), + End: testconvert.Time(time.Date(2022, 1, 2, 0, 0, 0, 0, time.UTC)), + }, + Metadata: &map[string]interface{}{"key": "value"}, TrackEvents: testconvert.Bool(true), Disable: testconvert.Bool(false), Version: testconvert.String("v1"), @@ -137,7 +135,7 @@ func TestConvertV1DtoToInternalFlag(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := tt.input.Convert(nil, "random-flag-name") + result := tt.input.Convert() assert.Equal(t, tt.expected, result) }) } diff --git a/model/dto/converter_v0.go b/model/dto/converter_v0.go deleted file mode 100644 index 3ddc14b460f..00000000000 --- a/model/dto/converter_v0.go +++ /dev/null @@ -1,332 +0,0 @@ -package dto - -import ( - "github.com/thomaspoignant/go-feature-flag/internal/flag" -) - -var ( - LegacyRuleName = "rule1" - defaultRuleName = "defaultRule" - - trueVariation = "True" - falseVariation = "False" - defaultVariation = "Default" - defaultPercentage = float64(0) - - emptyVarRes = "" - disableRuleValue = true - enableRuleValue = false -) - -// ConvertV0DtoToInternalFlag is converting a flag in the config file to the internal format. -// this function convert only the old format of the flag (before v1.0.0), to keep -// backward support of the configurations. -func ConvertV0DtoToInternalFlag(d DTO, isScheduledStep bool) flag.InternalFlag { - // Create variations based on the available definition in the flag v0 - var variations *map[string]*interface{} - newVariations := createVariationsV0(d, isScheduledStep) - if newVariations != nil { - variations = newVariations - } - - var rules *[]flag.Rule - var defaultRule *flag.Rule - if d.Rule != nil && *d.Rule != "" { - rules = &[]flag.Rule{createrule1(d)} - defaultRule = createDefaultrule1(d, true) - } else { - rules = nil - defaultRule = createDefaultrule1(d, false) - } - - internalFlag := flag.InternalFlag{ - Variations: variations, - Rules: rules, - DefaultRule: defaultRule, - TrackEvents: d.TrackEvents, - Disable: d.Disable, - Version: d.Version, - } - - var rollout *flag.Rollout - if d.Rollout != nil && (d.Rollout.Experimentation != nil || d.Rollout.V0Rollout.Scheduled != nil) { - rollout = convertRollout(d, internalFlag) - internalFlag.Scheduled = rollout.Scheduled - internalFlag.Experimentation = rollout.Experimentation - } - - return internalFlag -} - -// createDefaultrule1 create the default rule based on the legacy format. -func createDefaultrule1(d DTO, hasTargetRule bool) *flag.Rule { - hasProgressiveRollout := d.Rollout != nil && - d.Rollout.Progressive != nil && - d.Rollout.Progressive.ReleaseRamp.Start != nil && - d.Rollout.Progressive.ReleaseRamp.End != nil - - if hasProgressiveRollout && !hasTargetRule { - return &flag.Rule{ - Name: &defaultRuleName, - ProgressiveRollout: &flag.ProgressiveRollout{ - Initial: &flag.ProgressiveRolloutStep{ - Variation: &falseVariation, - Percentage: &d.Rollout.Progressive.Percentage.Initial, - Date: d.Rollout.Progressive.ReleaseRamp.Start, - }, - End: &flag.ProgressiveRolloutStep{ - Variation: &trueVariation, - Percentage: &d.Rollout.Progressive.Percentage.End, - Date: d.Rollout.Progressive.ReleaseRamp.End, - }, - }, - } - } - - if d.Rule == nil { - if d.Percentage == nil { - d.Percentage = &defaultPercentage - } - - p := computePercentages(*d.Percentage) - return &flag.Rule{ - Name: &defaultRuleName, - Percentages: p, - VariationResult: nil, - } - } - - return &flag.Rule{ - Name: &defaultRuleName, - VariationResult: &defaultVariation, - Percentages: nil, - } -} - -// createrule1 will create a rule based on the previous format -func createrule1(d DTO) flag.Rule { - // Handle the specific use case of progressive rollout. - var progressiveRollout *flag.ProgressiveRollout - if d.Rollout != nil && - d.Rollout.Progressive != nil && - d.Rollout.Progressive.ReleaseRamp.Start != nil && - d.Rollout.Progressive.ReleaseRamp.End != nil { - progressiveRollout = &flag.ProgressiveRollout{ - Initial: &flag.ProgressiveRolloutStep{ - Variation: &falseVariation, - Percentage: &d.Rollout.Progressive.Percentage.Initial, - Date: d.Rollout.Progressive.ReleaseRamp.Start, - }, - End: &flag.ProgressiveRolloutStep{ - Variation: &trueVariation, - Percentage: &d.Rollout.Progressive.Percentage.End, - Date: d.Rollout.Progressive.ReleaseRamp.End, - }, - } - } - - var percentages *map[string]float64 - if progressiveRollout == nil { - if d.Percentage != nil { - percentages = computePercentages(*d.Percentage) - } else { - percentages = &map[string]float64{ - trueVariation: 0, - falseVariation: 100, - } - } - } - - return flag.Rule{ - Name: &LegacyRuleName, - Query: d.Rule, - Percentages: percentages, - ProgressiveRollout: progressiveRollout, - } -} - -// createVariationsV0 will create a set of variations based on the previous format -func createVariationsV0(d DTO, isScheduleStep bool) *map[string]*interface{} { - variations := make(map[string]*interface{}, 3) - if d.True != nil { - variations[trueVariation] = d.True - } - if d.False != nil { - variations[falseVariation] = d.False - } - if d.Default != nil { - variations[defaultVariation] = d.Default - } - - if isScheduleStep && len(variations) == 0 { - return nil - } - return &variations -} - -// createScheduledStep is converting the old format of scheduled step to the new one. -// since the format has changed a lot the logic in this function is a bit complex to follow. -// In the tests we are testing that we have the same results as the one returns by the old -// flag logic. -func createScheduledStep(f flag.InternalFlag, dto ScheduledStepV0) flag.ScheduledStep { - step := flag.ScheduledStep{ - Date: dto.Date, - InternalFlag: flag.InternalFlag{ - Variations: createVariationsV0(dto.DTO, true), - }, - } - - legacyRuleIndex := f.GetRuleIndexByName(LegacyRuleName) - hasRuleBefore := legacyRuleIndex != nil && !f.GetRules()[*legacyRuleIndex].IsDisable() - updateRule := dto.Rule != nil - progressive := convertScheduledStepProgressiveRollout(dto) - - switch { - case hasRuleBefore && !updateRule: - if progressive != nil { - step.Rules = &[]flag.Rule{{Name: &LegacyRuleName, ProgressiveRollout: progressive}} - } else if dto.Percentage != nil { - step.Rules = &[]flag.Rule{{Name: &LegacyRuleName, Percentages: computePercentages(*dto.Percentage)}} - } - break - - case !hasRuleBefore && updateRule: - if *dto.Rule == "" { - step.Rules = &[]flag.Rule{{Name: &LegacyRuleName, Disable: &disableRuleValue}} - step.DefaultRule = &flag.Rule{Name: &defaultRuleName} - - if progressive != nil { - step.DefaultRule.ProgressiveRollout = progressive - } else if dto.Percentage != nil { - step.DefaultRule.Percentages = computePercentages(*dto.Percentage) - } - } else { - r := flag.Rule{Name: &LegacyRuleName, Disable: &enableRuleValue, Query: dto.Rule} - - switch { - case progressive != nil: - r.ProgressiveRollout = progressive - break - case dto.Percentage != nil: - r.Percentages = computePercentages(*dto.Percentage) - break - case f.DefaultRule != nil && f.DefaultRule.Percentages != nil && len(f.GetDefaultRule().GetPercentages()) > 0: - r.Percentages = deepCopyPercentages(f.GetDefaultRule().GetPercentages()) - break - default: - // no explicit percentage, default value is 0 - r.Percentages = computePercentages(0) - break - } - step.Rules = &[]flag.Rule{r} - - // clean up the default value - step.DefaultRule = &flag.Rule{ - Name: &defaultRuleName, - Percentages: &map[string]float64{trueVariation: -1, falseVariation: -1}, - VariationResult: &defaultVariation, - } - } - break - - case !hasRuleBefore && !updateRule: - if progressive != nil { - step.DefaultRule = &flag.Rule{VariationResult: &emptyVarRes, Name: &defaultRuleName, ProgressiveRollout: progressive} - } else if dto.Percentage != nil { - step.DefaultRule = &flag.Rule{ - VariationResult: &emptyVarRes, - Name: &defaultRuleName, - Percentages: computePercentages(*dto.Percentage), - } - } - break - - case hasRuleBefore && updateRule: - r := flag.Rule{Name: &LegacyRuleName, Query: dto.Rule, Disable: &enableRuleValue} - - if progressive != nil { - r.VariationResult = &emptyVarRes - r.ProgressiveRollout = progressive - } else if dto.Percentage != nil { - r.VariationResult = &emptyVarRes - r.Percentages = computePercentages(*dto.Percentage) - } - step.Rules = &[]flag.Rule{r} - break - } - - step.Disable = dto.Disable - step.TrackEvents = dto.TrackEvents - step.Version = dto.Version - return step -} - -func convertRollout(dto DTO, f flag.InternalFlag) *flag.Rollout { - r := flag.Rollout{} - if dto.Rollout.Experimentation != nil && - dto.Rollout.Experimentation.Start != nil && - dto.Rollout.Experimentation.End != nil { - r.Experimentation = &flag.ExperimentationRollout{ - Start: dto.Rollout.Experimentation.Start, - End: dto.Rollout.Experimentation.End, - } - } - - // it is not allowed to have a scheduled step inside a scheduled step - if dto.Rollout.V0Rollout.Scheduled != nil { - s := *dto.Rollout.V0Rollout.Scheduled - if s.Steps != nil { - var convertedSteps []flag.ScheduledStep - for _, v := range s.Steps { - convertedSteps = append(convertedSteps, createScheduledStep(f, v)) - } - r.Scheduled = &convertedSteps - } - } - return &r -} - -// computePercentages is creating the percentage structure based on the -// field percentage in the DTO. -func computePercentages(percentage float64) *map[string]float64 { - return &map[string]float64{ - trueVariation: percentage, - falseVariation: 100 - percentage, - } -} - -// deepCopyPercentages is creating a new map with the same values -func deepCopyPercentages(in map[string]float64) *map[string]float64 { - p := make(map[string]float64, len(in)) - // deep copy of the percentages to avoid being override - for k, v := range in { - p[k] = v - } - return &p -} - -// convertProgressiveRollout convert the legacy format to the new format. -// If we can't convert we return a nil value. -func convertScheduledStepProgressiveRollout(dto ScheduledStepV0) *flag.ProgressiveRollout { - hasProgressiveRollout := dto.Rollout != nil && - dto.Rollout.Progressive != nil && - dto.Rollout.Progressive.ReleaseRamp.End != nil && - dto.Rollout.Progressive.ReleaseRamp.Start != nil - - var progressive *flag.ProgressiveRollout - if hasProgressiveRollout { - progressive = &flag.ProgressiveRollout{ - Initial: &flag.ProgressiveRolloutStep{ - Variation: &falseVariation, - Percentage: &dto.Rollout.Progressive.Percentage.Initial, - Date: dto.Rollout.Progressive.ReleaseRamp.Start, - }, - End: &flag.ProgressiveRolloutStep{ - Variation: &trueVariation, - Percentage: &dto.Rollout.Progressive.Percentage.End, - Date: dto.Rollout.Progressive.ReleaseRamp.End, - }, - } - } - return progressive -} diff --git a/model/dto/dto.go b/model/dto/dto.go index 04c07c4aba8..88df35ab0a7 100644 --- a/model/dto/dto.go +++ b/model/dto/dto.go @@ -1,18 +1,12 @@ package dto import ( - "fmt" - "github.com/thomaspoignant/go-feature-flag/internal/flag" - "github.com/thomaspoignant/go-feature-flag/utils/fflog" ) // DTO is representing all the fields we can have in a flag. // This DTO supports all flag formats and convert them into an InternalFlag using a converter. type DTO struct { - DTOv1 `json:",inline" yaml:",inline" toml:",inline"` - DTOv0 `json:",inline" yaml:",inline" toml:",inline"` - // TrackEvents is false if you don't want to export the data in your data exporter. // Default value is true TrackEvents *bool `json:"trackEvents,omitempty" yaml:"trackEvents,omitempty" toml:"trackEvents,omitempty"` @@ -28,10 +22,7 @@ type DTO struct { // Converter (optional) is the name of converter to use, if no converter specified we try to determine // which converter to use based on the fields we receive for the flag Converter *string `json:"converter,omitempty" yaml:"converter,omitempty" toml:"converter,omitempty"` -} -// DTOv1 is the new format of the flags since version 1.X.X -type DTOv1 struct { // Variations are all the variations available for this flag. The minimum is 2 variations and, we don't have any max // limit except if the variationValue is a bool, the max is 2. Variations *map[string]*interface{} `json:"variations,omitempty" yaml:"variations,omitempty" toml:"variations,omitempty" jsonschema:"required,title=variations,description=All the variations available for this flag. You need at least 2 variations and it is a key value pair. All the variations should have the same type."` // nolint:lll @@ -61,42 +52,10 @@ type DTOv1 struct { Metadata *map[string]interface{} `json:"metadata,omitempty" yaml:"metadata,omitempty" toml:"metadata,omitempty" jsonschema:"title=metadata,description=A field containing information about your flag such as an issue tracker link a description etc..."` // nolint: lll } -// DTOv0 describe the fields of a flag. -type DTOv0 struct { - // Rule is the query use to select on which user the flag should apply. - // Rule format is based on the nikunjy/rules module. - // If no rule set, the flag apply to all users (percentage still apply). - Rule *string `json:"rule,omitempty" yaml:"rule,omitempty" toml:"rule,omitempty"` - - // Percentage of the users affected by the flag. - // Default value is 0 - Percentage *float64 `json:"percentage,omitempty" yaml:"percentage,omitempty" toml:"percentage,omitempty"` - - // True is the value return by the flag if apply to the user (rule is evaluated to true) - // and user is in the active percentage. - True *interface{} `json:"true,omitempty" yaml:"true,omitempty" toml:"true,omitempty"` - - // False is the value return by the flag if apply to the user (rule is evaluated to true) - // and user is not in the active percentage. - False *interface{} `json:"false,omitempty" yaml:"false,omitempty" toml:"false,omitempty"` - - // Default is the value return by the flag if not apply to the user (rule is evaluated to false). - Default *interface{} `json:"default,omitempty" yaml:"default,omitempty" toml:"default,omitempty"` - - // Rollout is the object to configure how the flag is rolled out. - // You have different rollout strategy available but only one is used at a time. - Rollout *Rollout `json:"rollout,omitempty" yaml:"rollout,omitempty" toml:"rollout,omitempty"` -} - -func (d *DTO) Convert(log *fflog.FFLogger, flagName string) flag.InternalFlag { +// Convert is converting the DTO into a flag.InternalFlag. +func (d *DTO) Convert() flag.InternalFlag { if d == nil || (DTO{}) == *d { return flag.InternalFlag{} } - if (d.Converter != nil && *d.Converter == "v0") || d.True != nil || d.False != nil { - log.Error(fmt.Sprintf("your flag %v is using GO Feature Flag flag format v0.x.x. The support of this "+ - "format will be removed soon, please consider migrating to the v1 format "+ - "(see https://gofeatureflag.org/docs/tooling/migrate_v0_v1).", flagName)) - return ConvertV0DtoToInternalFlag(*d, false) - } - return ConvertV1DtoToInternalFlag(*d) + return ConvertDtoToInternalFlag(*d) } diff --git a/testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml b/testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml index 1c15eb7be57..74aee1ba886 100644 --- a/testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml +++ b/testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml @@ -1,47 +1,82 @@ test-flag0: - percentage: 100 - true: true - false: false - default: false + variations: + Default: false + "False": false + "True": true + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag1: - percentage: 100 - true: "true" - false: "false" - default: "false" + variations: + Default: "false" + "False": "false" + "True": "true" + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag2: - percentage: 100 - true: 1 - false: 2 - default: 3 + variations: + Default: 3 + "False": 2 + "True": 1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag3: - percentage: 100 - true: - - yo - - ya - false: - - yo - - ya - default: - - yo - - ya + variations: + Default: + - yo + - ya + "False": + - yo + - ya + "True": + - yo + - ya + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag4: - percentage: 100 - true: - test: yo - false: - test: yo - default: - test: yo + variations: + Default: + test: yo + "False": + test: yo + "True": + test: yo + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag5: - percentage: 100 - true: 1.1 - false: 1.2 - default: 1.3 + variations: + Default: 1.3 + "False": 1.2 + "True": 1.1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 trackEvents: false test-flag6: - percentage: 100 - true: 1.1 - false: 1.2 - default: 1.3 + variations: + Default: 1.3 + "False": 1.2 + "True": 1.1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 trackEvents: false disable: true diff --git a/testdata/ffclient/all_flags/config_flag/flag-config-with-error.yaml b/testdata/ffclient/all_flags/config_flag/flag-config-with-error.yaml index c81e7937736..aac257d8f7c 100644 --- a/testdata/ffclient/all_flags/config_flag/flag-config-with-error.yaml +++ b/testdata/ffclient/all_flags/config_flag/flag-config-with-error.yaml @@ -1,40 +1,78 @@ test-flag0: - percentage: 100 - true: - false: false - default: false + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag1: - percentage: 100 - true: "true" - false: "false" - default: "false" + variations: + Default: "false" + "False": "false" + "True": "true" + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag2: - percentage: 100 - true: 1 - false: 2 - default: 3 + variations: + Default: 3 + "False": 2 + "True": 1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag3: - percentage: 100 - true: - - yo - - ya - false: - - yo - - ya - default: - - yo - - ya + variations: + Default: + - yo + - ya + "False": + - yo + - ya + "True": + - yo + - ya + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag4: - percentage: 100 - true: - test: yo - false: - test: yo - default: - test: yo + variations: + Default: + test: yo + "False": + test: yo + "True": + test: yo + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag5: - percentage: 100 - true: 1.1 - false: 1.2 - default: 1.3 + variations: + Default: 1.3 + "False": 1.2 + "True": 1.1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 trackEvents: false +test-flag6: + variations: + Default: 1.3 + "False": 1.2 + "True": 1.1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 + trackEvents: false + disable: true diff --git a/testdata/ffclient/all_flags/marshal_json/error_in_flag_0.json b/testdata/ffclient/all_flags/marshal_json/error_in_flag_0.json index a5b54ff093b..90106cd8941 100644 --- a/testdata/ffclient/all_flags/marshal_json/error_in_flag_0.json +++ b/testdata/ffclient/all_flags/marshal_json/error_in_flag_0.json @@ -1,19 +1,11 @@ { "flags": { - "test-flag0": { - "timestamp": 1622206239, - "variationType": "SdkDefault", - "trackEvents": true, - "reason":"ERROR", - "errorCode": "TYPE_MISMATCH", - "value": null - }, "test-flag1": { "value": "true", "timestamp": 1622206239, "variationType": "True", "trackEvents": true, - "reason":"STATIC", + "reason": "STATIC", "errorCode": "" }, "test-flag2": { @@ -21,7 +13,7 @@ "timestamp": 1622206239, "variationType": "True", "trackEvents": true, - "reason":"STATIC", + "reason": "STATIC", "errorCode": "" }, "test-flag3": { @@ -32,7 +24,7 @@ "timestamp": 1622206239, "variationType": "True", "trackEvents": true, - "reason":"STATIC", + "reason": "STATIC", "errorCode": "" }, "test-flag4": { @@ -42,7 +34,7 @@ "timestamp": 1622206239, "variationType": "True", "trackEvents": true, - "reason":"STATIC", + "reason": "STATIC", "errorCode": "" }, "test-flag5": { @@ -50,7 +42,7 @@ "timestamp": 1622206239, "variationType": "True", "trackEvents": false, - "reason":"STATIC", + "reason": "STATIC", "errorCode": "" } }, diff --git a/testdata/ffclient/get_flagstates/config_flag/flag-config-all-flags.yaml b/testdata/ffclient/get_flagstates/config_flag/flag-config-all-flags.yaml index 1c15eb7be57..74aee1ba886 100644 --- a/testdata/ffclient/get_flagstates/config_flag/flag-config-all-flags.yaml +++ b/testdata/ffclient/get_flagstates/config_flag/flag-config-all-flags.yaml @@ -1,47 +1,82 @@ test-flag0: - percentage: 100 - true: true - false: false - default: false + variations: + Default: false + "False": false + "True": true + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag1: - percentage: 100 - true: "true" - false: "false" - default: "false" + variations: + Default: "false" + "False": "false" + "True": "true" + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag2: - percentage: 100 - true: 1 - false: 2 - default: 3 + variations: + Default: 3 + "False": 2 + "True": 1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag3: - percentage: 100 - true: - - yo - - ya - false: - - yo - - ya - default: - - yo - - ya + variations: + Default: + - yo + - ya + "False": + - yo + - ya + "True": + - yo + - ya + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag4: - percentage: 100 - true: - test: yo - false: - test: yo - default: - test: yo + variations: + Default: + test: yo + "False": + test: yo + "True": + test: yo + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 test-flag5: - percentage: 100 - true: 1.1 - false: 1.2 - default: 1.3 + variations: + Default: 1.3 + "False": 1.2 + "True": 1.1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 trackEvents: false test-flag6: - percentage: 100 - true: 1.1 - false: 1.2 - default: 1.3 + variations: + Default: 1.3 + "False": 1.2 + "True": 1.1 + defaultRule: + name: legacyDefaultRule + percentage: + "False": 0 + "True": 100 trackEvents: false disable: true diff --git a/variation_all_flags_test.go b/variation_all_flags_test.go index 3042bdb706b..9579c942780 100644 --- a/variation_all_flags_test.go +++ b/variation_all_flags_test.go @@ -1,12 +1,14 @@ -package ffclient +package ffclient_test import ( "encoding/json" + "log/slog" "os" "testing" "time" "github.com/stretchr/testify/assert" + ffclient "github.com/thomaspoignant/go-feature-flag" "github.com/thomaspoignant/go-feature-flag/exporter/fileexporter" "github.com/thomaspoignant/go-feature-flag/ffcontext" "github.com/thomaspoignant/go-feature-flag/retriever/fileretriever" @@ -15,17 +17,18 @@ import ( func TestAllFlagsState(t *testing.T) { tests := []struct { name string - config Config + config ffclient.Config valid bool jsonOutput string initModule bool }{ { name: "Valid multiple types", - config: Config{ + config: ffclient.Config{ Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml", }, + LeveledLogger: slog.Default(), }, valid: true, jsonOutput: "./testdata/ffclient/all_flags/marshal_json/valid_multiple_types.json", @@ -33,7 +36,7 @@ func TestAllFlagsState(t *testing.T) { }, { name: "module not init", - config: Config{ + config: ffclient.Config{ Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml", }, @@ -44,7 +47,7 @@ func TestAllFlagsState(t *testing.T) { }, { name: "offline", - config: Config{ + config: ffclient.Config{ Offline: true, Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml", @@ -57,23 +60,22 @@ func TestAllFlagsState(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // init logger exportDir, _ := os.MkdirTemp("", "export") - tt.config.DataExporter = DataExporter{ + tt.config.DataExporter = ffclient.DataExporter{ FlushInterval: 1000, MaxEventInMemory: 1, Exporter: &fileexporter.Exporter{OutputDir: exportDir}, } - var goff *GoFeatureFlag + var goff *ffclient.GoFeatureFlag var err error if tt.initModule { - goff, err = New(tt.config) + goff, err = ffclient.New(tt.config) assert.NoError(t, err) defer goff.Close() } else { // we close directly so we can test with module not init - goff, _ = New(tt.config) + goff, _ = ffclient.New(tt.config) goff.Close() } @@ -109,7 +111,7 @@ func TestAllFlagsState(t *testing.T) { func TestGetFlagStates(t *testing.T) { tests := []struct { name string - config Config + config ffclient.Config valid bool jsonOutput string initModule bool @@ -117,7 +119,7 @@ func TestGetFlagStates(t *testing.T) { }{ { name: "Valid multiple flags", - config: Config{ + config: ffclient.Config{ Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/get_flagstates/config_flag/flag-config-all-flags.yaml", }, @@ -131,7 +133,7 @@ func TestGetFlagStates(t *testing.T) { }, { name: "empty list of flags in context", - config: Config{ + config: ffclient.Config{ Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/get_flagstates/config_flag/flag-config-all-flags.yaml", }, @@ -145,7 +147,7 @@ func TestGetFlagStates(t *testing.T) { }, { name: "no field in context context", - config: Config{ + config: ffclient.Config{ Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/get_flagstates/config_flag/flag-config-all-flags.yaml", }, @@ -157,7 +159,7 @@ func TestGetFlagStates(t *testing.T) { }, { name: "offline", - config: Config{ + config: ffclient.Config{ Offline: true, Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml", @@ -172,21 +174,21 @@ func TestGetFlagStates(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // init logger exportDir, _ := os.MkdirTemp("", "export") - tt.config.DataExporter = DataExporter{ + tt.config.DataExporter = ffclient.DataExporter{ FlushInterval: 1000, MaxEventInMemory: 1, Exporter: &fileexporter.Exporter{OutputDir: exportDir}, } - var goff *GoFeatureFlag + var goff *ffclient.GoFeatureFlag var err error if tt.initModule { - goff, err = New(tt.config) + goff, err = ffclient.New(tt.config) assert.NoError(t, err) defer goff.Close() } else { // we close directly so we can test with module not init - goff, _ = New(tt.config) + goff, _ = ffclient.New(tt.config) goff.Close() } @@ -221,21 +223,23 @@ func TestGetFlagStates(t *testing.T) { func TestAllFlagsFromCache(t *testing.T) { tests := []struct { name string - config Config + config ffclient.Config initModule bool + numberFlag int }{ { name: "Valid multiple types", - config: Config{ + config: ffclient.Config{ Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml", }, }, initModule: true, + numberFlag: 7, }, { name: "module not init", - config: Config{ + config: ffclient.Config{ Retriever: &fileretriever.Retriever{ Path: "./testdata/ffclient/all_flags/config_flag/flag-config-all-flags.yaml", }, @@ -245,21 +249,20 @@ func TestAllFlagsFromCache(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var goff *GoFeatureFlag + var goff *ffclient.GoFeatureFlag var err error if tt.initModule { - goff, err = New(tt.config) + goff, err = ffclient.New(tt.config) assert.NoError(t, err) defer goff.Close() flags, err := goff.GetFlagsFromCache() assert.NoError(t, err) - cf, _ := goff.cache.AllFlags() - assert.Equal(t, flags, cf) + assert.Equal(t, tt.numberFlag, len(flags)) } else { // we close directly so we can test with module not init - goff, _ = New(tt.config) + goff, _ = ffclient.New(tt.config) goff.Close() _, err := goff.GetFlagsFromCache()