Match on segments with no constraints (#66)

* Allow segments to match with no constraints
* Clear debug console response body on error

CHANGELOG.md

@@ -5,6 +5,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Fixed
+
+* Segments with no constraints now match all requests by default: [https://github.com/markphelps/flipt/issues/60](https://github.com/markphelps/flipt/issues/60)
+* Clear Debug Console response on error
+
 ## [v0.4.1](https://github.com/markphelps/flipt/releases/tag/v0.4.1) - 2019-05-11
 
 ### Added

docs/getting_started.md

@@ -70,7 +70,11 @@ You should see the message `Segment created!`.
 
 ### Create a Constraint
 
-Constraints determine which entities are members of which segments.
+Constraints are used to target a specific segment.
+
+!!! note
+    Constraints are not required to match a segment. A segment with no constraints will match every
+    request by default.
 
 To create a constraint:
 

docs/index.md

@@ -1,14 +1,14 @@
 # Flipt
 
-A self contained feature flag solution.
+A feature flag solution that runs in your existing infrastructure.
 
 ![Flipt](assets/images/flipt.png)
 
 ## What is Flipt
 
-Flipt is an open source, self contained application that enables you to use feature flags and experiment (A/B test) across services, running in **your** environment.
+Flipt is an open source feature flag application that allows you to run experiments across services in **your** environment.
 
-This means that you can deploy Flipt within your existing infrastructure and not have to worry about your information being sent to a third party, or the latency required to communicate across the internet.
+This means that you can deploy Flipt within your existing infrastructure and not have to worry about your information being sent to a third party or the latency required to communicate across the internet.
 
 Flipt includes native client SDKs as well as a REST API so you can choose how to best integrate Flipt with your applications.
 

storage/db_test.go

@@ -2,6 +2,7 @@ package storage
 
 import (
 	"database/sql"
+	"flag"
 	"fmt"
 	"os"
 	"testing"
@@ -17,7 +18,8 @@ import (
 
 var (
 	db     *sql.DB
-	logger logrus.FieldLogger
+	logger *logrus.Logger
+	debug  bool
 
 	flagStore    FlagStore
 	segmentStore SegmentStore
@@ -27,6 +29,8 @@ var (
 const testDBPath = "../flipt_test.db"
 
 func TestMain(m *testing.M) {
+	flag.BoolVar(&debug, "debug", false, "enable debug logging")
+	flag.Parse()
 	// os.Exit skips defer calls
 	// so we need to use another fn
 	os.Exit(run(m))
@@ -37,6 +41,10 @@ func run(m *testing.M) int {
 
 	logger = logrus.New()
 
+	if debug {
+		logger.SetLevel(logrus.DebugLevel)
+	}
+
 	db, err = sql.Open("sqlite3", fmt.Sprintf("%s?_fk=true", testDBPath))
 	if err != nil {
 		logger.Fatal(err)

storage/rule.go

@@ -426,6 +426,14 @@ func (s *RuleStorage) distributions(ctx context.Context, rule *flipt.Rule) (err
 	return rows.Err()
 }
 
+type optionalConstraint struct {
+	ID       sql.NullString
+	Type     sql.NullInt64
+	Property sql.NullString
+	Operator sql.NullString
+	Value    sql.NullString
+}
+
 type constraint struct {
 	Type     flipt.ComparisonType
 	Property string
@@ -484,10 +492,10 @@ func (s *RuleStorage) Evaluate(ctx context.Context, r *flipt.EvaluationRequest)
 		return resp, ErrInvalidf("flag %q is disabled", r.FlagKey)
 	}
 
-	// get all rules for flag with their constraints
-	rows, err := s.builder.Select("r.id, r.flag_key, r.segment_key, r.rank, c.type, c.property, c.operator, c.value").
+	// get all rules for flag with their constraints if any
+	rows, err := s.builder.Select("r.id, r.flag_key, r.segment_key, r.rank, c.id, c.type, c.property, c.operator, c.value").
 		From("rules r").
-		Join("constraints c ON (r.segment_key = c.segment_key)").
+		LeftJoin("constraints c ON (r.segment_key = c.segment_key)").
 		Where(sq.Eq{"r.flag_key": r.FlagKey}).
 		OrderBy("r.rank ASC").
 		GroupBy("r.id").
@@ -507,27 +515,45 @@ func (s *RuleStorage) Evaluate(ctx context.Context, r *flipt.EvaluationRequest)
 
 	for rows.Next() {
 		var (
-			existingRule   *rule
-			tempRule       rule
-			tempConstraint constraint
+			existingRule       *rule
+			tempRule           rule
+			optionalConstraint optionalConstraint
 		)
 
-		if err := rows.Scan(&tempRule.ID, &tempRule.FlagKey, &tempRule.SegmentKey, &tempRule.Rank, &tempConstraint.Type, &tempConstraint.Property, &tempConstraint.Operator, &tempConstraint.Value); err != nil {
+		if err := rows.Scan(&tempRule.ID, &tempRule.FlagKey, &tempRule.SegmentKey, &tempRule.Rank, &optionalConstraint.ID, &optionalConstraint.Type, &optionalConstraint.Property, &optionalConstraint.Operator, &optionalConstraint.Value); err != nil {
 			return resp, err
 		}
 
 		// current rule we know about
 		if existingRule != nil && existingRule.ID == tempRule.ID {
-			existingRule.Constraints = append(existingRule.Constraints, tempConstraint)
+			if optionalConstraint.ID.Valid {
+				constraint := constraint{
+					Type:     flipt.ComparisonType(optionalConstraint.Type.Int64),
+					Property: optionalConstraint.Property.String,
+					Operator: optionalConstraint.Operator.String,
+					Value:    optionalConstraint.Value.String,
+				}
+				existingRule.Constraints = append(existingRule.Constraints, constraint)
+			}
 		} else {
 			// haven't seen this rule before
 			existingRule = &rule{
-				ID:          tempRule.ID,
-				FlagKey:     tempRule.FlagKey,
-				SegmentKey:  tempRule.SegmentKey,
-				Rank:        tempRule.Rank,
-				Constraints: []constraint{tempConstraint},
+				ID:         tempRule.ID,
+				FlagKey:    tempRule.FlagKey,
+				SegmentKey: tempRule.SegmentKey,
+				Rank:       tempRule.Rank,
 			}
+
+			if optionalConstraint.ID.Valid {
+				constraint := constraint{
+					Type:     flipt.ComparisonType(optionalConstraint.Type.Int64),
+					Property: optionalConstraint.Property.String,
+					Operator: optionalConstraint.Operator.String,
+					Value:    optionalConstraint.Value.String,
+				}
+				existingRule.Constraints = append(existingRule.Constraints, constraint)
+			}
+
 			rules = append(rules, *existingRule)
 		}
 	}
@@ -537,6 +563,7 @@ func (s *RuleStorage) Evaluate(ctx context.Context, r *flipt.EvaluationRequest)
 	}
 
 	if len(rules) == 0 {
+		logger.Debug("no rules match")
 		return resp, nil
 	}
 
@@ -573,7 +600,7 @@ func (s *RuleStorage) Evaluate(ctx context.Context, r *flipt.EvaluationRequest)
 			// constraint doesn't match, we can short circuit and move to the next rule
 			// because we must match ALL constraints
 			if !match {
-				logger.Debug("does not match")
+				logger.Debugf("constraint: %+v does not match", c)
 				break
 			}
 

storage/rule_test.go

@@ -845,7 +845,7 @@ func TestEvaluate_RolloutDistribution(t *testing.T) {
 			FlagKey:   flag.Key,
 			RuleId:    rule.Id,
 			VariantId: variants[1].Id,
-			Rollout:   10,
+			Rollout:   50,
 		},
 	} {
 		_, err := ruleStore.CreateDistribution(context.TODO(), req)
@@ -915,7 +915,128 @@ func TestEvaluate_RolloutDistribution(t *testing.T) {
 	}
 }
 
-func Test_validateConstraint(t *testing.T) {
+func TestEvaluate_NoConstraints(t *testing.T) {
+	flag, err := flagStore.CreateFlag(context.TODO(), &flipt.CreateFlagRequest{
+		Key:         t.Name(),
+		Name:        t.Name(),
+		Description: "foo",
+		Enabled:     true,
+	})
+
+	require.NoError(t, err)
+
+	var variants []*flipt.Variant
+
+	for _, req := range []*flipt.CreateVariantRequest{
+		{
+			FlagKey: flag.Key,
+			Key:     fmt.Sprintf("foo_%s", t.Name()),
+		},
+		{
+			FlagKey: flag.Key,
+			Key:     fmt.Sprintf("bar_%s", t.Name()),
+		},
+	} {
+		variant, err := flagStore.CreateVariant(context.TODO(), req)
+		require.NoError(t, err)
+		variants = append(variants, variant)
+	}
+
+	segment, err := segmentStore.CreateSegment(context.TODO(), &flipt.CreateSegmentRequest{
+		Key:         t.Name(),
+		Name:        t.Name(),
+		Description: "foo",
+	})
+
+	require.NoError(t, err)
+
+	rule, err := ruleStore.CreateRule(context.TODO(), &flipt.CreateRuleRequest{
+		FlagKey:    flag.Key,
+		SegmentKey: segment.Key,
+	})
+
+	require.NoError(t, err)
+
+	for _, req := range []*flipt.CreateDistributionRequest{
+		{
+			FlagKey:   flag.Key,
+			RuleId:    rule.Id,
+			VariantId: variants[0].Id,
+			Rollout:   50,
+		},
+		{
+			FlagKey:   flag.Key,
+			RuleId:    rule.Id,
+			VariantId: variants[1].Id,
+			Rollout:   50,
+		},
+	} {
+		_, err := ruleStore.CreateDistribution(context.TODO(), req)
+		require.NoError(t, err)
+	}
+
+	tests := []struct {
+		name              string
+		req               *flipt.EvaluationRequest
+		matchesVariantKey string
+		wantMatch         bool
+	}{
+		{
+			name: "match no value - variant 1",
+			req: &flipt.EvaluationRequest{
+				FlagKey:  flag.Key,
+				EntityId: "01",
+				Context:  map[string]string{},
+			},
+			matchesVariantKey: variants[0].Key,
+			wantMatch:         true,
+		},
+		{
+			name: "match no value - variant 2",
+			req: &flipt.EvaluationRequest{
+				FlagKey:  flag.Key,
+				EntityId: "10",
+				Context:  map[string]string{},
+			},
+			matchesVariantKey: variants[1].Key,
+			wantMatch:         true,
+		},
+		{
+			name: "match string value - variant 2",
+			req: &flipt.EvaluationRequest{
+				FlagKey:  flag.Key,
+				EntityId: "10",
+				Context: map[string]string{
+					"bar": "boz",
+				},
+			},
+			matchesVariantKey: variants[1].Key,
+			wantMatch:         true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			resp, err := ruleStore.Evaluate(context.TODO(), tt.req)
+			require.NoError(t, err)
+			assert.NotNil(t, resp)
+			assert.Equal(t, flag.Key, resp.FlagKey)
+			assert.Equal(t, tt.req.Context, resp.RequestContext)
+
+			if !tt.wantMatch {
+				assert.False(t, resp.Match)
+				assert.Empty(t, resp.SegmentKey)
+				return
+			}
+
+			assert.True(t, resp.Match)
+			assert.Equal(t, segment.Key, resp.SegmentKey)
+			assert.Equal(t, tt.matchesVariantKey, resp.Value)
+		})
+	}
+}
+
+func Test_validate(t *testing.T) {
 	tests := []struct {
 		name       string
 		constraint constraint
@@ -1414,7 +1535,7 @@ func Test_evaluate(t *testing.T) {
 		assert.Equal(t, "one", d.VariantKey)
 	})
 
-	t.Run("33/0 inside", func(t *testing.T) {
+	t.Run("33/0 match", func(t *testing.T) {
 		var (
 			distributions = []distribution{{VariantKey: "one"}}
 			buckets       = []int{333}
@@ -1429,7 +1550,7 @@ func Test_evaluate(t *testing.T) {
 		assert.Equal(t, "one", d.VariantKey)
 	})
 
-	t.Run("33/0 outside", func(t *testing.T) {
+	t.Run("33/0 no match", func(t *testing.T) {
 		var (
 			distributions = []distribution{{VariantKey: "one"}}
 			buckets       = []int{333}

ui/src/components/Flags/DebugConsole.vue

@@ -98,7 +98,7 @@ export default {
           this.response = response.data;
         })
         .catch(error => {
-          this.$set(this.response, "error", error.response.data.error);
+          this.response = { error: error.response.data.error };
         });
     }
   }