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

fix: updated merge functionality #182

Merged
merged 2 commits into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ install-mockgen:
go install github.com/golang/mock/[email protected]
mockgen: install-mockgen
mockgen -source=pkg/sync/http_sync.go -destination=pkg/sync/mock/http.go -package=syncmock
mockgen -source=pkg/eval/ievaluator.go -destination=pkg/eval/mock/ievaluator.go -package=evalmock
2 changes: 1 addition & 1 deletion pkg/eval/ievaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ do parsing and validation of the flag state and evaluate flags in response to ha
*/
type IEvaluator interface {
GetState() (string, error)
SetState(state string) error
SetState(source string, state string) error

ResolveBooleanValue(
flagKey string,
Expand Down
5 changes: 3 additions & 2 deletions pkg/eval/json_evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (je *JSONEvaluator) GetState() (string, error) {
return string(data), nil
}

func (je *JSONEvaluator) SetState(state string) error {
func (je *JSONEvaluator) SetState(source string, state string) error {
schemaLoader := gojsonschema.NewStringLoader(schema.FlagdDefinitions)
flagStringLoader := gojsonschema.NewStringLoader(state)
result, err := gojsonschema.Validate(schemaLoader, flagStringLoader)
Expand All @@ -64,7 +64,8 @@ func (je *JSONEvaluator) SetState(state string) error {
if err := validateDefaultVariants(newFlags); err != nil {
return err
}
je.state = je.state.Merge(newFlags)

je.state = je.state.Merge(source, newFlags)

return nil
}
Expand Down
10 changes: 9 additions & 1 deletion pkg/eval/json_evaluator_model.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,19 @@ type Evaluators struct {
Evaluators map[string]json.RawMessage `json:"$evaluators"`
}

func (f Flags) Merge(ff Flags) Flags {
func (f Flags) Merge(source string, ff Flags) Flags {
result := Flags{Flags: make(map[string]Flag)}
for k, v := range f.Flags {
if v.Source == source {
if _, ok := ff.Flags[k]; !ok {
// flag has been deleted
continue
}
}
result.Flags[k] = v
}
for k, v := range ff.Flags {
v.Source = source
result.Flags[k] = v
}
return result
Expand All @@ -28,4 +35,5 @@ type Flag struct {
DefaultVariant string `json:"defaultVariant"`
Variants map[string]any `json:"variants"`
Targeting json.RawMessage `json:"targeting"`
Source string `json:"source"`
}
54 changes: 34 additions & 20 deletions pkg/eval/json_evaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ var Flags = fmt.Sprintf(`{

func TestGetState_Valid_ContainsFlag(t *testing.T) {
evaluator := eval.JSONEvaluator{Logger: l}
err := evaluator.SetState(ValidFlags)
err := evaluator.SetState("", ValidFlags)
if err != nil {
t.Fatalf("Expected no error")
}
Expand All @@ -302,7 +302,7 @@ func TestSetState_Invalid_Error(t *testing.T) {
evaluator := eval.JSONEvaluator{Logger: l}

// set state with an invalid flag definition
err := evaluator.SetState(InvalidFlags)
err := evaluator.SetState("", InvalidFlags)
if err == nil {
t.Fatalf("Expected error")
}
Expand All @@ -312,7 +312,7 @@ func TestSetState_Valid_NoError(t *testing.T) {
evaluator := eval.JSONEvaluator{Logger: l}

// set state with a valid flag definition
err := evaluator.SetState(ValidFlags)
err := evaluator.SetState("", ValidFlags)
if err != nil {
t.Fatalf("Expected no error")
}
Expand All @@ -334,7 +334,7 @@ func TestResolveBooleanValue(t *testing.T) {
}

evaluator := eval.JSONEvaluator{Logger: l}
err := evaluator.SetState(Flags)
err := evaluator.SetState("", Flags)
if err != nil {
t.Fatalf("Expected no error")
}
Expand Down Expand Up @@ -373,7 +373,7 @@ func TestResolveStringValue(t *testing.T) {
}

evaluator := eval.JSONEvaluator{Logger: l}
err := evaluator.SetState(Flags)
err := evaluator.SetState("", Flags)
if err != nil {
t.Fatalf("Expected no error")
}
Expand Down Expand Up @@ -413,7 +413,7 @@ func TestResolveFloatValue(t *testing.T) {
}

evaluator := eval.JSONEvaluator{Logger: l}
err := evaluator.SetState(Flags)
err := evaluator.SetState("", Flags)
if err != nil {
t.Fatalf("Expected no error")
}
Expand Down Expand Up @@ -453,7 +453,7 @@ func TestResolveIntValue(t *testing.T) {
}

evaluator := eval.JSONEvaluator{Logger: l}
err := evaluator.SetState(Flags)
err := evaluator.SetState("", Flags)
if err != nil {
t.Fatalf("Expected no error")
}
Expand Down Expand Up @@ -493,7 +493,7 @@ func TestResolveObjectValue(t *testing.T) {
}

evaluator := eval.JSONEvaluator{Logger: l}
err := evaluator.SetState(Flags)
err := evaluator.SetState("", Flags)
if err != nil {
t.Fatalf("Expected no error")
}
Expand Down Expand Up @@ -523,10 +523,11 @@ func TestResolveObjectValue(t *testing.T) {
func TestMergeFlags(t *testing.T) {
t.Parallel()
tests := []struct {
name string
current eval.Flags
new eval.Flags
want eval.Flags
name string
current eval.Flags
new eval.Flags
newSource string
want eval.Flags
}{
{
name: "both nil",
Expand Down Expand Up @@ -555,14 +556,26 @@ func TestMergeFlags(t *testing.T) {
{
name: "extra fields on each",
current: eval.Flags{Flags: map[string]eval.Flag{
"waka": {DefaultVariant: "off"},
"waka": {
DefaultVariant: "off",
Source: "1",
},
}},
new: eval.Flags{Flags: map[string]eval.Flag{
"paka": {DefaultVariant: "on"},
"paka": {
DefaultVariant: "on",
},
}},
newSource: "2",
want: eval.Flags{Flags: map[string]eval.Flag{
"waka": {DefaultVariant: "off"},
"paka": {DefaultVariant: "on"},
"waka": {
DefaultVariant: "off",
Source: "1",
},
"paka": {
DefaultVariant: "on",
Source: "2",
},
}},
},
{
Expand Down Expand Up @@ -597,8 +610,8 @@ func TestMergeFlags(t *testing.T) {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := tt.current.Merge(tt.new)
require.Equal(t, got, tt.want)
got := tt.current.Merge(tt.newSource, tt.new)
require.Equal(t, tt.want, got)
})
}
}
Expand Down Expand Up @@ -656,7 +669,7 @@ func TestSetState_DefaultVariantValidation(t *testing.T) {
t.Run(name, func(t *testing.T) {
jsonEvaluator := eval.JSONEvaluator{}

err := jsonEvaluator.SetState(tt.jsonFlags)
err := jsonEvaluator.SetState("", tt.jsonFlags)

if tt.valid && err != nil {
t.Error(err)
Expand Down Expand Up @@ -714,6 +727,7 @@ func TestState_Evaluator(t *testing.T) {
},
"defaultVariant": "recursive",
"state": "ENABLED",
"source":"",
"targeting": {
"if": [
{
Expand Down Expand Up @@ -792,7 +806,7 @@ func TestState_Evaluator(t *testing.T) {
t.Run(name, func(t *testing.T) {
jsonEvaluator := eval.JSONEvaluator{}

err := jsonEvaluator.SetState(tt.inputState)
err := jsonEvaluator.SetState("", tt.inputState)
if err != nil {
if !tt.expectedError {
t.Error(err)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (r *Runtime) updateState(ctx context.Context, syncr sync.ISync) error {
}
r.mu.Lock()
defer r.mu.Unlock()
err = r.Evaluator.SetState(msg)
err = r.Evaluator.SetState(syncr.Source(), msg)
if err != nil {
return fmt.Errorf("set state: %w", err)
}
Expand Down
23 changes: 12 additions & 11 deletions pkg/service/connect_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/bufbuild/connect-go"
"github.com/golang/mock/gomock"
mock "github.com/open-feature/flagd/pkg/eval/mock"
"github.com/open-feature/flagd/pkg/model"
service "github.com/open-feature/flagd/pkg/service"
log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -75,7 +76,7 @@ func TestConnectService_UnixConnection(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctrl := gomock.NewController(t)
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveBooleanValue(tt.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -167,7 +168,7 @@ func TestConnectService_ResolveBoolean(t *testing.T) {
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveBooleanValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -214,7 +215,7 @@ func BenchmarkConnectService_ResolveBoolean(b *testing.B) {
},
}
for name, tt := range tests {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveBooleanValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -299,7 +300,7 @@ func TestConnectService_ResolveString(t *testing.T) {
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveStringValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -346,7 +347,7 @@ func BenchmarkConnectService_ResolveString(b *testing.B) {
},
}
for name, tt := range tests {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveStringValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -431,7 +432,7 @@ func TestConnectService_ResolveFloat(t *testing.T) {
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveFloatValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -478,7 +479,7 @@ func BenchmarkConnectService_ResolveFloat(b *testing.B) {
},
}
for name, tt := range tests {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveFloatValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -563,7 +564,7 @@ func TestConnectService_ResolveInt(t *testing.T) {
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveIntValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -610,7 +611,7 @@ func BenchmarkConnectService_ResolveInt(b *testing.B) {
},
}
for name, tt := range tests {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveIntValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -699,7 +700,7 @@ func TestConnectService_ResolveObject(t *testing.T) {
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveObjectValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down Expand Up @@ -756,7 +757,7 @@ func BenchmarkConnectService_ResolveObject(b *testing.B) {
},
}
for name, tt := range tests {
eval := NewMockIEvaluator(ctrl)
eval := mock.NewMockIEvaluator(ctrl)
eval.EXPECT().ResolveObjectValue(tt.functionArgs.req.FlagKey, gomock.Any()).Return(
tt.evalFields.result,
tt.evalFields.variant,
Expand Down
4 changes: 4 additions & 0 deletions pkg/sync/filepath_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ type FilePathSync struct {
ProviderArgs ProviderArgs
}

func (fs *FilePathSync) Source() string {
return fs.URI
}

func (fs *FilePathSync) Fetch(_ context.Context) (string, error) {
if fs.URI == "" {
return "", errors.New("no filepath string set")
Expand Down
4 changes: 4 additions & 0 deletions pkg/sync/http_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ type Cron interface {
Start()
}

func (fs *HTTPSync) Source() string {
return fs.URI
}

func (fs *HTTPSync) fetchBodyFromURL(ctx context.Context, url string) ([]byte, error) {
req, err := http.NewRequestWithContext(ctx, "GET", url, bytes.NewBuffer(nil))
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/sync/isync.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ type ProviderArgs map[string]string
type ISync interface {
Fetch(ctx context.Context) (string, error)
Notify(ctx context.Context, c chan<- INotify)
Source() string
}
Loading