more uts, enable integration

splitio · Nov 11, 2023 · 12fd3b0 · 12fd3b0
1 parent c114120
commit 12fd3b0
Show file tree

Hide file tree

Showing 6 changed files with 139 additions and 13 deletions.
diff --git 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
@@ -4,14 +4,14 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"slices"
 	"strconv"
 	"strings"
 
 	"github.com/gin-gonic/gin"
 	"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
@@ -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
@@ -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
@@ -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
@@ -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),
 	)
 }