From 12fd3b005df5922f8fea929e09106f44522f3991 Mon Sep 17 00:00:00 2001 From: Martin Redolatti <martin.redolatti@split.io> Date: Sat, 11 Nov 2023 19:45:34 -0300 Subject: [PATCH] more uts, enable integration --- splitio/commitversion.go | 2 +- splitio/proxy/controllers/sdk.go | 12 ++- splitio/proxy/controllers/sdk_test.go | 113 ++++++++++++++++++++++++++ splitio/proxy/flagsets/flagsets.go | 19 ++++- splitio/proxy/initialization.go | 2 + splitio/proxy/proxy.go | 4 +- 6 files changed, 139 insertions(+), 13 deletions(-) diff --git a/splitio/commitversion.go b/splitio/commitversion.go index 3f4f64b9..d25d07f9 100644 --- a/splitio/commitversion.go +++ b/splitio/commitversion.go @@ -5,4 +5,4 @@ This file is created automatically, please do not edit */ // CommitVersion is the version of the last commit previous to release -const CommitVersion = "ef035eb" +const CommitVersion = "c114120" diff --git a/splitio/proxy/controllers/sdk.go b/splitio/proxy/controllers/sdk.go index def1e11a..f689cd6c 100644 --- a/splitio/proxy/controllers/sdk.go +++ b/splitio/proxy/controllers/sdk.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "net/http" - "slices" "strconv" "strings" @@ -12,6 +11,7 @@ import ( "github.com/splitio/go-split-commons/v5/dtos" "github.com/splitio/go-split-commons/v5/service" "github.com/splitio/go-toolkit/v5/logging" + "golang.org/x/exp/slices" "github.com/splitio/split-synchronizer/v5/splitio/proxy/caching" "github.com/splitio/split-synchronizer/v5/splitio/proxy/flagsets" @@ -60,14 +60,12 @@ func (c *SdkServerController) SplitChanges(ctx *gin.Context) { since = -1 } - sets := strings.Split(ctx.Query("sets"), ",") - if !slices.IsSorted(sets) { - c.logger.Warning(fmt.Sprintf("SDK [%s] is sending flagsets unordered.", ctx.Request.Header.Get("SplitSDKVersion"))) // TODO(mredolatti): get this header properly - slices.Sort(sets) + rawSets := strings.Split(ctx.Query("sets"), ",") + sets := c.fsmatcher.Sanitize(rawSets) + if !slices.Equal(sets, rawSets) { + c.logger.Warning(fmt.Sprintf("SDK [%s] is sending flagsets unordered or with duplicates.", ctx.Request.Header.Get("SplitSDKVersion"))) } - sets = c.fsmatcher.Sanitize(sets) - c.logger.Debug(fmt.Sprintf("SDK Fetches Feature Flags Since: %d", since)) splits, err := c.fetchSplitChangesSince(since, sets) diff --git a/splitio/proxy/controllers/sdk_test.go b/splitio/proxy/controllers/sdk_test.go index 450a1c57..bf49a3a3 100644 --- a/splitio/proxy/controllers/sdk_test.go +++ b/splitio/proxy/controllers/sdk_test.go @@ -13,6 +13,7 @@ import ( "github.com/splitio/go-split-commons/v5/service" "github.com/splitio/go-split-commons/v5/service/mocks" "github.com/splitio/go-toolkit/v5/logging" + "github.com/stretchr/testify/assert" "github.com/splitio/split-synchronizer/v5/splitio/proxy/flagsets" "github.com/splitio/split-synchronizer/v5/splitio/proxy/storage" @@ -141,6 +142,118 @@ func TestSplitChangesNonCachedRecipe(t *testing.T) { } } +func TestSplitChangesWithFlagSets(t *testing.T) { + gin.SetMode(gin.TestMode) + resp := httptest.NewRecorder() + ctx, router := gin.CreateTestContext(resp) + + logger := logging.NewLogger(nil) + + group := router.Group("/api") + controller := NewSdkServerController( + logger, + &mocks.MockSplitFetcher{ + FetchCall: func(changeNumber int64, fetchOptions *service.FetchOptions) (*dtos.SplitChangesDTO, error) { + t.Error("should not be called") + return nil, nil + }, + }, + &psmocks.ProxySplitStorageMock{ + ChangesSinceCall: func(since int64, sets []string) (*dtos.SplitChangesDTO, error) { + assert.Equal(t, []string{"a", "b", "c"}, sets) // sets should be passed already sorted + return &dtos.SplitChangesDTO{ + Since: -1, + Till: 1, + Splits: []dtos.SplitDTO{ + {Name: "s1"}, + {Name: "s2"}, + }, + }, nil + }, + RegisterOlderCnCall: func(payload *dtos.SplitChangesDTO) { + t.Error("should not be called") + }, + }, + nil, + flagsets.NewMatcher(false, nil), + ) + controller.Register(group) + + ctx.Request, _ = http.NewRequest(http.MethodGet, "/api/splitChanges?since=-1&sets=c,b,b,a", nil) + ctx.Request.Header.Set("Authorization", "Bearer someApiKey") + ctx.Request.Header.Set("SplitSDKVersion", "go-1.1.1") + ctx.Request.Header.Set("SplitSDKMachineIp", "1.2.3.4") + ctx.Request.Header.Set("SplitSDKMachineName", "ip-1-2-3-4") + router.ServeHTTP(resp, ctx.Request) + + assert.Equal(t, 200, resp.Code) + + body, err := ioutil.ReadAll(resp.Body) + assert.Nil(t, err) + + var s dtos.SplitChangesDTO + assert.Nil(t, json.Unmarshal(body, &s)) + assert.Equal(t, 2, len(s.Splits)) + assert.Equal(t, int64(-1), s.Since) + assert.Equal(t, int64(1), s.Till) +} + +func TestSplitChangesWithFlagSetsStrict(t *testing.T) { + gin.SetMode(gin.TestMode) + resp := httptest.NewRecorder() + ctx, router := gin.CreateTestContext(resp) + + logger := logging.NewLogger(nil) + + group := router.Group("/api") + controller := NewSdkServerController( + logger, + &mocks.MockSplitFetcher{ + FetchCall: func(changeNumber int64, fetchOptions *service.FetchOptions) (*dtos.SplitChangesDTO, error) { + t.Error("should not be called") + return nil, nil + }, + }, + &psmocks.ProxySplitStorageMock{ + ChangesSinceCall: func(since int64, sets []string) (*dtos.SplitChangesDTO, error) { + assert.Equal(t, []string{"a", "c"}, sets) // sets should be passed already sorted + return &dtos.SplitChangesDTO{ + Since: -1, + Till: 1, + Splits: []dtos.SplitDTO{ + {Name: "s1"}, + {Name: "s2"}, + }, + }, nil + }, + RegisterOlderCnCall: func(payload *dtos.SplitChangesDTO) { + t.Error("should not be called") + }, + }, + nil, + flagsets.NewMatcher(true, []string{"a", "c"}), + ) + controller.Register(group) + + ctx.Request, _ = http.NewRequest(http.MethodGet, "/api/splitChanges?since=-1&sets=c,b,b,a", nil) + ctx.Request.Header.Set("Authorization", "Bearer someApiKey") + ctx.Request.Header.Set("SplitSDKVersion", "go-1.1.1") + ctx.Request.Header.Set("SplitSDKMachineIp", "1.2.3.4") + ctx.Request.Header.Set("SplitSDKMachineName", "ip-1-2-3-4") + router.ServeHTTP(resp, ctx.Request) + + assert.Equal(t, 200, resp.Code) + + body, err := ioutil.ReadAll(resp.Body) + assert.Nil(t, err) + + var s dtos.SplitChangesDTO + assert.Nil(t, json.Unmarshal(body, &s)) + assert.Equal(t, 2, len(s.Splits)) + assert.Equal(t, int64(-1), s.Since) + assert.Equal(t, int64(1), s.Till) +} + func TestSplitChangesNonCachedRecipeAndFetchFails(t *testing.T) { gin.SetMode(gin.TestMode) resp := httptest.NewRecorder() diff --git a/splitio/proxy/flagsets/flagsets.go b/splitio/proxy/flagsets/flagsets.go index 0419a66c..52115660 100644 --- a/splitio/proxy/flagsets/flagsets.go +++ b/splitio/proxy/flagsets/flagsets.go @@ -1,5 +1,7 @@ package flagsets +import "golang.org/x/exp/slices" + type FlagSetMatcher struct { strict bool sets map[string]struct{} @@ -18,18 +20,29 @@ func NewMatcher(strict bool, fetched []string) FlagSetMatcher { return out } +// Sort, Dedupe & Filter input flagsets. returns sanitized list and a boolean indicating whether a sort was necessary func (f *FlagSetMatcher) Sanitize(input []string) []string { - if !f.strict || len(input) == 0 { + if len(input) == 0 { return input } - for idx := range input { - if _, ok := f.sets[input[idx]]; !ok { + seen := map[string]struct{}{} + for idx := 0; idx < len(input); idx++ { // cant use range because we're srhinking the slice inside the loop + item := input[idx] + if (f.strict && !setContains(f.sets, item)) || setContains(seen, item) { if idx+1 < len(input) { input[idx] = input[len(input)-1] } input = input[:len(input)-1] } + seen[item] = struct{}{} } + + slices.Sort(input) return input } + +func setContains(set map[string]struct{}, item string) bool { + _, ok := set[item] + return ok +} diff --git a/splitio/proxy/initialization.go b/splitio/proxy/initialization.go index fab1011c..3293e9e7 100644 --- a/splitio/proxy/initialization.go +++ b/splitio/proxy/initialization.go @@ -252,6 +252,8 @@ func Start(logger logging.LoggerInterface, cfg *pconf.Main) error { Telemetry: localTelemetryStorage, Cache: httpCache, TLSConfig: tlsConfig, + FlagSets: cfg.FlagSetsFilter, + FlagSetsStrictMatching: cfg.FlagSetStrictMatching, } if ilcfg := cfg.Integrations.ImpressionListener; ilcfg.Endpoint != "" { diff --git a/splitio/proxy/proxy.go b/splitio/proxy/proxy.go index 274f6a76..4aab8ea5 100644 --- a/splitio/proxy/proxy.go +++ b/splitio/proxy/proxy.go @@ -82,7 +82,7 @@ type Options struct { FlagSets []string - FlagSetsStrictMatchibg bool + FlagSetsStrictMatching bool } // API bundles all components required to answer API calls from Split sdks @@ -159,7 +159,7 @@ func setupSdkController(options *Options) *controllers.SdkServerController { options.SplitFetcher, options.ProxySplitStorage, options.ProxySegmentStorage, - flagsets.NewMatcher(options.FlagSetsStrictMatchibg, options.FlagSets), + flagsets.NewMatcher(options.FlagSetsStrictMatching, options.FlagSets), ) }