polishing

cmd/proxy/main.go

@@ -34,7 +34,10 @@ func setupConfig(cliArgs *cconf.CliFlags) (*conf.Main, error) {
 	}
 
 	cconf.PopulateFromArguments(&proxyConf, cliArgs.RawConfig)
-	return &proxyConf, nil
+
+	var err error
+	proxyConf.FlagSetsFilter, err = cconf.ValidateFlagsets(proxyConf.FlagSetsFilter)
+	return &proxyConf, err
 }
 
 func main() {
@@ -57,8 +60,13 @@ func main() {
 
 	cfg, err := setupConfig(cliArgs)
 	if err != nil {
-		fmt.Println("error processing config: ", err)
-		os.Exit(exitCodeConfigError)
+		var fsErr cconf.FlagSetValidationError
+		if errors.As(err, &fsErr) {
+			fmt.Println("error processing flagsets: ", err.Error())
+		} else {
+			fmt.Println("error processing config: ", err)
+			os.Exit(exitCodeConfigError)
+		}
 	}
 
 	logger := log.BuildFromConfig(&cfg.Logging, "Split-Proxy", &cfg.Integrations.Slack)

cmd/synchronizer/main.go

@@ -3,9 +3,7 @@ package main
 import (
 	"errors"
 	"fmt"
-	"github.com/splitio/go-split-commons/v5/flagsets"
 	"os"
-	"strings"
 
 	"github.com/splitio/split-synchronizer/v5/splitio"
 	"github.com/splitio/split-synchronizer/v5/splitio/common"
@@ -24,18 +22,6 @@ func parseCliArgs() *cconf.CliFlags {
 	return cconf.ParseCliArgs(&conf.Main{})
 }
 
-type flagSetValidationError struct {
-	wrapped []error
-}
-
-func (f flagSetValidationError) Error() string {
-	var errors []string
-	for _, err := range f.wrapped {
-		errors = append(errors, err.Error())
-	}
-	return strings.Join(errors, ".|| ")
-}
-
 func setupConfig(cliArgs *cconf.CliFlags) (*conf.Main, error) {
 	syncConf := conf.Main{}
 	cconf.PopulateDefaults(&syncConf)
@@ -50,13 +36,7 @@ func setupConfig(cliArgs *cconf.CliFlags) (*conf.Main, error) {
 	cconf.PopulateFromArguments(&syncConf, cliArgs.RawConfig)
 
 	var err error
-	sanitizedFlagSets, fsErr := flagsets.SanitizeMany(syncConf.FlagSetsFilter)
-	if fsErr != nil {
-		err = flagSetValidationError{wrapped: fsErr}
-	}
-	if sanitizedFlagSets != nil {
-		syncConf.FlagSetsFilter = sanitizedFlagSets
-	}
+	syncConf.FlagSetsFilter, err = cconf.ValidateFlagsets(syncConf.FlagSetsFilter)
 	return &syncConf, err
 }
 
@@ -80,7 +60,7 @@ func main() {
 
 	cfg, err := setupConfig(cliArgs)
 	if err != nil {
-		var fsErr flagSetValidationError
+		var fsErr cconf.FlagSetValidationError
 		if errors.As(err, &fsErr) {
 			fmt.Println("error processing flagset: ", err.Error())
 		} else {

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 = "0484f05"
+const CommitVersion = "5b37ef1"

splitio/common/conf/validators.go

@@ -0,0 +1,28 @@
+package conf
+
+import (
+	"strings"
+
+	"github.com/splitio/go-split-commons/v5/flagsets"
+)
+
+type FlagSetValidationError struct {
+	wrapped []error
+}
+
+func (f FlagSetValidationError) Error() string {
+	var errors []string
+	for _, err := range f.wrapped {
+		errors = append(errors, err.Error())
+	}
+	return strings.Join(errors, ".|| ")
+}
+
+func ValidateFlagsets(sets []string) ([]string, error) {
+	var toRet error
+	sanitizedFlagSets, fsErr := flagsets.SanitizeMany(sets)
+	if fsErr != nil {
+		toRet = FlagSetValidationError{wrapped: fsErr}
+	}
+	return sanitizedFlagSets, toRet
+}

splitio/common/conf/validators_test.go

@@ -0,0 +1,24 @@
+package conf
+
+import (
+	"testing"
+
+	"github.com/splitio/go-split-commons/v5/dtos"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFlagSetValidationError(t *testing.T) {
+
+	sanitized, err := ValidateFlagsets([]string{"Flagset1", " flagset2 ", "123#@flagset"})
+	assert.NotNil(t, err)
+	assert.Equal(t, []string{"flagset1", "flagset2"}, sanitized)
+
+	asFVE := err.(FlagSetValidationError)
+	assert.Equal(t, 3, len(asFVE.wrapped))
+	assert.ElementsMatch(t, []error{
+		dtos.FlagSetValidatonError{Message: "Flag Set name Flagset1 should be all lowercase - converting string to lowercase"},
+		dtos.FlagSetValidatonError{Message: "Flag Set name  flagset2  has extra whitespace, trimming"},
+		dtos.FlagSetValidatonError{Message: "you passed 123#@flagset, Flag Set must adhere to the regular expressions ^[a-z0-9][_a-z0-9]{0,49}$. This means a Flag Set must " +
+			"start with a letter or number, be in lowercase, alphanumeric and have a max length of 50 characters. 123#@flagset was discarded."},
+	}, asFVE.wrapped)
+}

splitio/producer/initialization.go

@@ -123,7 +123,7 @@ func Start(logger logging.LoggerInterface, cfg *conf.Main) error {
 	eventEvictionMonitor := evcalc.New(1)
 
 	workers := synchronizer.Workers{
-		SplitUpdater: split.NewSplitUpdater(storages.SplitStorage, splitAPI.SplitFetcher, logger, syncTelemetryStorage, appMonitor, flagsets.NewFlagSetFilter(nil)), // TODO(mredolatti)
+		SplitUpdater: split.NewSplitUpdater(storages.SplitStorage, splitAPI.SplitFetcher, logger, syncTelemetryStorage, appMonitor, flagSetsFilter),
 		SegmentUpdater: segment.NewSegmentUpdater(storages.SplitStorage, storages.SegmentStorage, splitAPI.SegmentFetcher,
 			logger, syncTelemetryStorage, appMonitor),
 		ImpressionsCountRecorder: impressionscount.NewRecorderSingle(impressionsCounter, splitAPI.ImpressionRecorder,

splitio/proxy/caching/workers.go

@@ -31,7 +31,7 @@ func NewCacheAwareSplitSync(
 	flagSetsFilter flagsets.FlagSetFilter,
 ) *CacheAwareSplitSynchronizer {
 	return &CacheAwareSplitSynchronizer{
-		wrapped:      split.NewSplitUpdater(splitStorage, splitFetcher, logger, runtimeTelemetry, appMonitor, flagsets.NewFlagSetFilter(nil)), // TODO(mredolatti): fix this
+		wrapped:      split.NewSplitUpdater(splitStorage, splitFetcher, logger, runtimeTelemetry, appMonitor, flagSetsFilter),
 		splitStorage: splitStorage,
 		cacheFlusher: cacheFlusher,
 	}

splitio/proxy/controllers/sdk.go

@@ -140,11 +140,7 @@ func (c *SdkServerController) fetchSplitChangesSince(since int64, sets []string)
 
 	// perform a fetch to the BE using the supplied `since`, have the storage process it's response &, retry
 	// TODO(mredolatti): implement basic collapsing here to avoid flooding the BE with requests
-	fetchOptions := service.NewFetchOptions(true, nil) // TODO: pass the configured sets if any
-	splits, err = c.fetcher.Fetch(since, &fetchOptions)
-	if err != nil {
-		return nil, fmt.Errorf("error fetching splitChanges for an older since: %w", err)
-	}
-	c.proxySplitStorage.RegisterOlderCn(splits)
-	return c.proxySplitStorage.ChangesSince(since, sets)
+	fetchOptions := service.NewFetchOptions(true, nil)
+	fetchOptions.FlagSetsFilter = strings.Join(sets, ",") // at this point the sets have been sanitized & sorted
+	return c.fetcher.Fetch(since, &fetchOptions)
 }

splitio/proxy/controllers/sdk_test.go

@@ -61,6 +61,9 @@ func TestSplitChangesRecentSince(t *testing.T) {
 	assert.Equal(t, 2, len(s.Splits))
 	assert.Equal(t, int64(-1), s.Since)
 	assert.Equal(t, int64(1), s.Till)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
 }
 
 func TestSplitChangesOlderSince(t *testing.T) {
@@ -70,11 +73,6 @@ func TestSplitChangesOlderSince(t *testing.T) {
 	splitStorage.On("ChangesSince", int64(-1), []string(nil)).
 		Return((*dtos.SplitChangesDTO)(nil), storage.ErrSinceParamTooOld).
 		Once()
-	splitStorage.On("ChangesSince", int64(-1), []string(nil)).
-		Return(&dtos.SplitChangesDTO{Since: -1, Till: 1, Splits: []dtos.SplitDTO{{Name: "s1", Status: "ACTIVE"}, {Name: "s2", Status: "ACTIVE"}}}, nil).
-		Once()
-	splitStorage.On("RegisterOlderCn", &dtos.SplitChangesDTO{Since: -1, Till: 1, Splits: []dtos.SplitDTO{{Name: "s1", Status: "ACTIVE"}, {Name: "s2", Status: "ACTIVE"}}}).
-		Once()
 
 	var splitFetcher splitFetcherMock
 	splitFetcher.On("Fetch", int64(-1), ref(service.NewFetchOptions(true, nil))).
@@ -114,6 +112,9 @@ func TestSplitChangesOlderSince(t *testing.T) {
 	assert.Equal(t, 2, len(s.Splits))
 	assert.Equal(t, int64(-1), s.Since)
 	assert.Equal(t, int64(1), s.Till)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
 }
 
 func TestSplitChangesOlderSinceFetchFails(t *testing.T) {
@@ -152,6 +153,9 @@ func TestSplitChangesOlderSinceFetchFails(t *testing.T) {
 	router.ServeHTTP(resp, ctx.Request)
 
 	assert.Equal(t, 500, resp.Code)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
 }
 
 func TestSplitChangesWithFlagSets(t *testing.T) {
@@ -196,6 +200,9 @@ func TestSplitChangesWithFlagSets(t *testing.T) {
 	assert.Equal(t, 2, len(s.Splits))
 	assert.Equal(t, int64(-1), s.Since)
 	assert.Equal(t, int64(1), s.Till)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
 }
 
 func TestSplitChangesWithFlagSetsStrict(t *testing.T) {
@@ -240,6 +247,9 @@ func TestSplitChangesWithFlagSetsStrict(t *testing.T) {
 	assert.Equal(t, 2, len(s.Splits))
 	assert.Equal(t, int64(-1), s.Since)
 	assert.Equal(t, int64(1), s.Till)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
 }
 
 func TestSegmentChanges(t *testing.T) {
@@ -278,6 +288,10 @@ func TestSegmentChanges(t *testing.T) {
 	assert.Nil(t, err)
 
 	assert.Equal(t, dtos.SegmentChangesDTO{Name: "someSegment", Added: []string{"k1", "k2"}, Removed: []string{}, Since: -1, Till: 1}, s)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
+	segmentStorage.AssertExpectations(t)
 }
 
 func TestSegmentChangesNotFound(t *testing.T) {
@@ -306,6 +320,10 @@ func TestSegmentChangesNotFound(t *testing.T) {
 	ctx.Request.Header.Set("SplitSDKMachineName", "ip-1-2-3-4")
 	router.ServeHTTP(resp, ctx.Request)
 	assert.Equal(t, 404, resp.Code)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
+	segmentStorage.AssertExpectations(t)
 }
 
 func TestMySegments(t *testing.T) {
@@ -343,6 +361,10 @@ func TestMySegments(t *testing.T) {
 	assert.Nil(t, err)
 
 	assert.Equal(t, MSC{MySegments: []dtos.MySegmentDTO{{Name: "segment1"}, {Name: "segment2"}}}, ms)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
+	segmentStorage.AssertExpectations(t)
 }
 
 func TestMySegmentsError(t *testing.T) {
@@ -371,6 +393,10 @@ func TestMySegmentsError(t *testing.T) {
 	ctx.Request.Header.Set("SplitSDKMachineName", "ip-1-2-3-4")
 	router.ServeHTTP(resp, ctx.Request)
 	assert.Equal(t, 500, resp.Code)
+
+	splitStorage.AssertExpectations(t)
+	splitFetcher.AssertExpectations(t)
+	segmentStorage.AssertExpectations(t)
 }
 
 type splitFetcherMock struct {