Add unssuported matcher logic in NewSplit and tets cases

splitio · May 10, 2024 · 82c933c · 82c933c
1 parent f3ccef6
commit 82c933c
Show file tree

Hide file tree

Showing 5 changed files with 273 additions and 20 deletions.
diff --git a/engine/grammar/condition.go b/engine/grammar/condition.go
@@ -18,17 +18,14 @@ type Condition struct {
 }
 
 // NewCondition instantiates a new Condition struct with appropriate wrappers around dtos and returns it.
-func NewCondition(cond *dtos.ConditionDTO, ctx *injection.Context, logger logging.LoggerInterface) *Condition {
+func NewCondition(cond *dtos.ConditionDTO, ctx *injection.Context, logger logging.LoggerInterface) (*Condition, error) {
 	partitions := make([]Partition, 0)
 	for _, part := range cond.Partitions {
 		partitions = append(partitions, Partition{partitionData: part})
 	}
-	matcherObjs := make([]matchers.MatcherInterface, 0)
-	for _, matcher := range cond.MatcherGroup.Matchers {
-		m, err := matchers.BuildMatcher(&matcher, ctx, logger)
-		if err == nil {
-			matcherObjs = append(matcherObjs, m)
-		}
+	matcherObjs, err := processMatchers(cond.MatcherGroup.Matchers, ctx, logger)
+	if err != nil {
+		return nil, err
 	}
 
 	return &Condition{
@@ -37,19 +34,22 @@ func NewCondition(cond *dtos.ConditionDTO, ctx *injection.Context, logger loggin
 		partitions:    partitions,
 		label:         cond.Label,
 		conditionType: cond.ConditionType,
-	}
+	}, nil
 }
 
-func proccessMatchers(condMatchers []dtos.MatcherDTO, ctx *injection.Context, logger logging.LoggerInterface) []matchers.MatcherInterface {
+func processMatchers(condMatchers []dtos.MatcherDTO, ctx *injection.Context, logger logging.LoggerInterface) ([]matchers.MatcherInterface, error) {
 	matcherObjs := make([]matchers.MatcherInterface, 0)
 	for _, matcher := range condMatchers {
 		m, err := matchers.BuildMatcher(&matcher, ctx, logger)
 		if err == nil {
 			matcherObjs = append(matcherObjs, m)
-		} else if errType, ok := err.(datatypes.UnsupportedMatcherError); ok {
-
+		} else {
+			if _, ok := err.(datatypes.UnsupportedMatcherError); ok {
+				return nil, err
+			}
 		}
 	}
+	return matcherObjs, nil
 }
 
 // Partition struct with added logic that wraps around a DTO

diff --git a/engine/grammar/condition_test.go b/engine/grammar/condition_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/splitio/go-split-commons/v5/dtos"
+	"github.com/splitio/go-split-commons/v5/engine/grammar/matchers"
 
 	"github.com/splitio/go-toolkit/v5/logging"
 )
@@ -46,7 +47,11 @@ func TestConditionWrapperObject(t *testing.T) {
 		},
 	}
 
-	wrapped := NewCondition(&condition1, nil, logger)
+	wrapped, err := NewCondition(&condition1, nil, logger)
+
+	if err != nil {
+		t.Error("err should be nil")
+	}
 
 	if wrapped.Label() != "Label1" {
 		t.Error("Label not set properly")
@@ -113,7 +118,10 @@ func TestAnotherWrapper(t *testing.T) {
 		},
 	}
 
-	wrapped := NewCondition(&condition1, nil, logger)
+	wrapped, err := NewCondition(&condition1, nil, logger)
+	if err != nil {
+		t.Error("err should be nil")
+	}
 
 	if wrapped.Label() != "Label2" {
 		t.Error("Label not set properly")
@@ -141,3 +149,90 @@ func TestAnotherWrapper(t *testing.T) {
 		t.Error("CalculateTreatment returned incorrect treatment")
 	}
 }
+
+func TestConditionUnsupportedMatcherWrapperObject(t *testing.T) {
+	logger := logging.NewLogger(&logging.LoggerOptions{})
+	condition1 := dtos.ConditionDTO{
+		ConditionType: "WHITELIST",
+		Label:         "Label1",
+		MatcherGroup: dtos.MatcherGroupDTO{
+			Combiner: "AND",
+			Matchers: []dtos.MatcherDTO{
+				{
+					Negate:      false,
+					MatcherType: "UNSUPPORTED",
+					KeySelector: &dtos.KeySelectorDTO{
+						Attribute:   nil,
+						TrafficType: "something",
+					},
+				},
+				{
+					Negate:      true,
+					MatcherType: "ALL_KEYS",
+					KeySelector: &dtos.KeySelectorDTO{
+						Attribute:   nil,
+						TrafficType: "something",
+					},
+				},
+			},
+		},
+		Partitions: []dtos.PartitionDTO{
+			{
+				Size:      75,
+				Treatment: "on",
+			},
+			{
+				Size:      25,
+				Treatment: "off",
+			},
+		},
+	}
+
+	_, err := NewCondition(&condition1, nil, logger)
+
+	if err == nil {
+		t.Error("err should not be nil")
+	}
+}
+
+func TestConditionMatcherWithNilStringWrapperObject(t *testing.T) {
+	logger := logging.NewLogger(&logging.LoggerOptions{})
+	condition1 := dtos.ConditionDTO{
+		ConditionType: "WHITELIST",
+		Label:         "Label1",
+		MatcherGroup: dtos.MatcherGroupDTO{
+			Combiner: "AND",
+			Matchers: []dtos.MatcherDTO{
+				{
+					Negate:      false,
+					MatcherType: matchers.MatcherTypeStartsWith,
+					KeySelector: &dtos.KeySelectorDTO{
+						Attribute:   nil,
+						TrafficType: "something",
+					},
+					Whitelist: nil,
+				},
+			},
+		},
+		Partitions: []dtos.PartitionDTO{
+			{
+				Size:      75,
+				Treatment: "on",
+			},
+			{
+				Size:      25,
+				Treatment: "off",
+			},
+		},
+	}
+
+	condition, err := NewCondition(&condition1, nil, logger)
+
+	if err != nil {
+		t.Error("err should be nil")
+	}
+
+	if len(condition.matchers) != 0 {
+		t.Error("matchers should be empty")
+	}
+}
diff --git a/engine/grammar/split.go b/engine/grammar/split.go
@@ -2,6 +2,8 @@ package grammar
 
 import (
 	"github.com/splitio/go-split-commons/v5/dtos"
+	"github.com/splitio/go-split-commons/v5/engine/evaluator/impressionlabels"
+	"github.com/splitio/go-split-commons/v5/engine/grammar/matchers"
 
 	"github.com/splitio/go-toolkit/v5/injection"
 	"github.com/splitio/go-toolkit/v5/logging"
@@ -13,21 +15,35 @@ type Split struct {
 	conditions []*Condition
 }
 
+var conditionReplacementUnsupportedMatcher []*Condition = []*Condition{{
+	conditionType: ConditionTypeWhitelist,
+	label:         impressionlabels.UnsupportedMatcherType,
+	partitions:    []Partition{{partitionData: dtos.PartitionDTO{Treatment: "control", Size: 100}}},
+	matchers:      []matchers.MatcherInterface{matchers.NewAllKeysMatcher(false)},
+}}
+
 // NewSplit instantiates a new Split object and all it's internal structures mapped to model classes
 func NewSplit(splitDTO *dtos.SplitDTO, ctx *injection.Context, logger logging.LoggerInterface) *Split {
-	conditions := make([]*Condition, 0)
-	for _, cond := range splitDTO.Conditions {
-		conditions = append(conditions, NewCondition(&cond, ctx, logger))
-	}
-
 	split := Split{
-		conditions: conditions,
+		conditions: processConditions(splitDTO.Conditions, splitDTO, ctx, logger),
 		splitData:  splitDTO,
 	}
 
 	return &split
 }
 
+func processConditions(conditions []dtos.ConditionDTO, splitDTO *dtos.SplitDTO, ctx *injection.Context, logger logging.LoggerInterface) []*Condition {
+	conditionsToReturn := make([]*Condition, 0)
+	for _, cond := range splitDTO.Conditions {
+		condition, err := NewCondition(&cond, ctx, logger)
+		if err != nil {
+			return conditionReplacementUnsupportedMatcher
+		}
+		conditionsToReturn = append(conditionsToReturn, condition)
+	}
+	return conditionsToReturn
+}
+
 // Name returns the name of the feature
 func (s *Split) Name() string {
 	return s.splitData.Name

diff --git a/engine/grammar/split_test.go b/engine/grammar/split_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/splitio/go-split-commons/v5/dtos"
+	"github.com/splitio/go-split-commons/v5/engine/grammar/matchers"
 
 	"github.com/splitio/go-toolkit/v5/logging"
 )
@@ -57,3 +58,143 @@ func TestSplitCreation(t *testing.T) {
 		t.Error("Traffic allocation should be 100")
 	}
 }
+
+func TestSplitCreationWithConditionsMatcher(t *testing.T) {
+	logger := logging.NewLogger(&logging.LoggerOptions{})
+	dto := dtos.SplitDTO{
+		Algo:         1,
+		ChangeNumber: 123,
+		Conditions: []dtos.ConditionDTO{{
+			ConditionType: ConditionTypeWhitelist,
+			Label:         "test",
+			Partitions:    []dtos.PartitionDTO{{Treatment: "off", Size: 100}},
+			MatcherGroup: dtos.MatcherGroupDTO{
+				Combiner: "AND",
+				Matchers: []dtos.MatcherDTO{{MatcherType: matchers.MatcherTypeAllKeys, Negate: false}},
+			},
+		}, {
+			ConditionType: ConditionTypeWhitelist,
+			Label:         "test",
+			Partitions:    []dtos.PartitionDTO{{Treatment: "on", Size: 100}},
+			MatcherGroup: dtos.MatcherGroupDTO{
+				Combiner: "AND",
+				Matchers: []dtos.MatcherDTO{{MatcherType: matchers.MatcherTypeAllKeys, Negate: false}},
+			},
+		}},
+		DefaultTreatment:      "def",
+		Killed:                false,
+		Name:                  "split1",
+		Seed:                  1234,
+		Status:                "ACTIVE",
+		TrafficAllocation:     100,
+		TrafficAllocationSeed: 333,
+		TrafficTypeName:       "tt1",
+	}
+	split := NewSplit(&dto, nil, logger)
+
+	if split.Algo() != SplitAlgoLegacy {
+		t.Error("Algo() should return legacy")
+	}
+
+	if split.ChangeNumber() != 123 {
+		t.Error("Incorrect changenumber returned")
+	}
+
+	if split.DefaultTreatment() != "def" {
+		t.Error("Incorrect default treatment")
+	}
+
+	if split.Killed() {
+		t.Error("Split should not be marked as killed")
+	}
+
+	if split.Seed() != 1234 {
+		t.Error("Incorrect seed")
+	}
+
+	if split.Name() != "split1" {
+		t.Error("Incorrect split name")
+	}
+
+	if split.Status() != SplitStatusActive {
+		t.Error("Status should be active")
+	}
+
+	if split.TrafficAllocation() != 100 {
+		t.Error("Traffic allocation should be 100")
+	}
+
+	if len(split.conditions) != 2 {
+		t.Error("conditions length should be 2")
+	}
+}
+
+func TestSplitCreationWithUnsupportedMatcher(t *testing.T) {
+	logger := logging.NewLogger(&logging.LoggerOptions{})
+	dto := dtos.SplitDTO{
+		Algo:         1,
+		ChangeNumber: 123,
+		Conditions: []dtos.ConditionDTO{{
+			ConditionType: ConditionTypeWhitelist,
+			Label:         "test",
+			Partitions:    []dtos.PartitionDTO{{Treatment: "control", Size: 100}},
+			MatcherGroup: dtos.MatcherGroupDTO{
+				Combiner: "AND",
+				Matchers: []dtos.MatcherDTO{{MatcherType: "unssuported", Negate: false}},
+			},
+		}, {
+			ConditionType: ConditionTypeWhitelist,
+			Label:         "test",
+			Partitions:    []dtos.PartitionDTO{{Treatment: "on", Size: 100}},
+			MatcherGroup: dtos.MatcherGroupDTO{
+				Combiner: "AND",
+				Matchers: []dtos.MatcherDTO{{MatcherType: matchers.MatcherTypeAllKeys, Negate: false}},
+			},
+		}},
+		DefaultTreatment:      "def",
+		Killed:                false,
+		Name:                  "split1",
+		Seed:                  1234,
+		Status:                "ACTIVE",
+		TrafficAllocation:     100,
+		TrafficAllocationSeed: 333,
+		TrafficTypeName:       "tt1",
+	}
+	split := NewSplit(&dto, nil, logger)
+
+	if split.Algo() != SplitAlgoLegacy {
+		t.Error("Algo() should return legacy")
+	}
+
+	if split.ChangeNumber() != 123 {
+		t.Error("Incorrect changenumber returned")
+	}
+
+	if split.DefaultTreatment() != "def" {
+		t.Error("Incorrect default treatment")
+	}
+
+	if split.Killed() {
+		t.Error("Split should not be marked as killed")
+	}
+
+	if split.Seed() != 1234 {
+		t.Error("Incorrect seed")
+	}
+
+	if split.Name() != "split1" {
+		t.Error("Incorrect split name")
+	}
+
+	if split.Status() != SplitStatusActive {
+		t.Error("Status should be active")
+	}
+
+	if split.TrafficAllocation() != 100 {
+		t.Error("Traffic allocation should be 100")
+	}
+
+	if len(split.conditions) != 1 {
+		t.Error("conditions length should be 1")
+	}
+}
diff --git a/engine/validator/matchers.go b/engine/validator/matchers.go
@@ -6,6 +6,7 @@ import (
 	"github.com/splitio/go-split-commons/v5/engine/evaluator/impressionlabels"
 	"github.com/splitio/go-split-commons/v5/engine/grammar"
 	"github.com/splitio/go-split-commons/v5/engine/grammar/matchers"
+	"github.com/splitio/go-split-commons/v5/engine/grammar/matchers/datatypes"
 	"github.com/splitio/go-toolkit/v5/injection"
 	"github.com/splitio/go-toolkit/v5/logging"
 )
@@ -25,7 +26,7 @@ func shouldOverrideConditions(conditions []dtos.ConditionDTO, logger logging.Log
 	for _, condition := range conditions {
 		for _, matcher := range condition.MatcherGroup.Matchers {
 			_, err := matchers.BuildMatcher(&matcher, &injection.Context{}, logger)
-			if err != nil {
+			if _, ok := err.(datatypes.UnsupportedMatcherError); ok {
 				return true
 			}
 		}