Merge pull request #1919 from flipt-io/optional-constraint

chore(rollouts): account for segments without constraints

build/testing/integration/readonly/readonly_test.go

@@ -101,7 +101,7 @@ func TestReadOnly(t *testing.T) {
 				NamespaceKey: namespace,
 			})
 			require.NoError(t, err)
-			require.Len(t, flags.Flags, 52)
+			require.Len(t, flags.Flags, 53)
 
 			flag := flags.Flags[0]
 			assert.Equal(t, namespace, flag.NamespaceKey)
@@ -128,8 +128,8 @@ func TestReadOnly(t *testing.T) {
 					require.NoError(t, err)
 
 					if flags.NextPageToken == "" {
-						// ensure last page contains 2 entries (boolean and disabled)
-						assert.Len(t, flags.Flags, 2)
+						// ensure last page contains 3 entries (boolean and disabled)
+						assert.Len(t, flags.Flags, 3)
 
 						found = append(found, flags.Flags...)
 
@@ -144,7 +144,7 @@ func TestReadOnly(t *testing.T) {
 					nextPage = flags.NextPageToken
 				}
 
-				require.Len(t, found, 52)
+				require.Len(t, found, 53)
 			})
 		})
 
@@ -189,7 +189,7 @@ func TestReadOnly(t *testing.T) {
 			})
 
 			require.NoError(t, err)
-			require.Len(t, segments.Segments, 50)
+			require.Len(t, segments.Segments, 51)
 
 			t.Run("Paginated (page size 10)", func(t *testing.T) {
 				var (
@@ -205,18 +205,20 @@ func TestReadOnly(t *testing.T) {
 					require.NoError(t, err)
 
 					// ensure each page is of length 10
-					assert.Len(t, segments.Segments, 10)
 
 					found = append(found, segments.Segments...)
 
 					if segments.NextPageToken == "" {
+						assert.Len(t, segments.Segments, 1)
 						break
 					}
 
+					assert.Len(t, segments.Segments, 10)
+
 					nextPage = segments.NextPageToken
 				}
 
-				require.Len(t, found, 50)
+				require.Len(t, found, 51)
 			})
 		})
 
@@ -530,6 +532,18 @@ func TestReadOnly(t *testing.T) {
 					assert.Equal(t, evaluation.EvaluationReason_MATCH_EVALUATION_REASON, result.Reason)
 					assert.True(t, result.Enabled, "segment evaluation value should be true")
 				})
+
+				t.Run("segment with no constraints", func(t *testing.T) {
+					result, err := sdk.Evaluation().Boolean(ctx, &evaluation.EvaluationRequest{
+						NamespaceKey: namespace,
+						FlagKey:      "flag_boolean_no_constraints",
+					})
+
+					require.NoError(t, err)
+
+					assert.Equal(t, evaluation.EvaluationReason_MATCH_EVALUATION_REASON, result.Reason)
+					assert.True(t, result.Enabled, "segment evaluation value should be true")
+				})
 			})
 
 			t.Run("Batch", func(t *testing.T) {

build/testing/integration/readonly/testdata/default.yaml

@@ -15579,6 +15579,16 @@ flags:
   type: VARIANT_FLAG_TYPE
   description: Disabled Flag Description
   enabled: false
+- key: flag_boolean_no_constraints
+  name: FLAG_NO_CONSTRAINTS
+  type: BOOLEAN_FLAG_TYPE
+  description: Flag Boolean no segment constraints
+  enabled: false
+  rollouts:
+  - description: enabled for segment_no_constraints
+    segment:
+      key: segment_no_constraints
+      value: true
 segments:
 - key: segment_001
   name: SEGMENT_001
@@ -16230,3 +16240,6 @@ segments:
     operator: eq
     value: segment_050
   match_type: ALL_MATCH_TYPE
+- key: segment_no_constraints
+  name: SEGMENT_NO_CONSTRAINTS
+  description: Some Segment Description

build/testing/integration/readonly/testdata/production.yaml

@@ -15580,6 +15580,16 @@ flags:
   type: VARIANT_FLAG_TYPE
   description: Disabled Flag Description
   enabled: false
+- key: flag_boolean_no_constraints
+  name: FLAG_NO_CONSTRAINTS
+  type: BOOLEAN_FLAG_TYPE
+  description: Flag Boolean no segment constraints
+  enabled: false
+  rollouts:
+  - description: enabled for segment_no_constraints
+    segment:
+      key: segment_no_constraints
+      value: true
 segments:
 - key: segment_001
   name: SEGMENT_001
@@ -16231,3 +16241,6 @@ segments:
     operator: eq
     value: segment_050
   match_type: ALL_MATCH_TYPE
+- key: segment_no_constraints
+  name: SEGMENT_NO_CONSTRAINTS
+  description: Some Segment Description

internal/storage/sql/common/evaluation.go

@@ -196,7 +196,7 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey
 			c.value AS constraint_value
 		FROM rollout_segments rs
 		JOIN segments s ON (rs.segment_key = s."key")
-		JOIN constraints c ON (rs.segment_key = c.segment_key)
+		LEFT JOIN constraints c ON (rs.segment_key = c.segment_key)
 	) rss ON (r.id = rss.rollout_id)
 	`).
 		Where(sq.And{sq.Eq{"r.namespace_key": namespaceKey}, sq.Eq{"r.flag_key": flagKey}}).
@@ -219,17 +219,14 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey
 
 	for rows.Next() {
 		var (
-			rolloutId            string
-			evaluationRollout    storage.EvaluationRollout
-			rtPercentageNumber   sql.NullFloat64
-			rtPercentageValue    sql.NullBool
-			rsSegmentKey         sql.NullString
-			rsSegmentValue       sql.NullBool
-			rsMatchType          sql.NullInt32
-			rsConstraintType     sql.NullInt32
-			rsConstraintProperty sql.NullString
-			rsConstraintOperator sql.NullString
-			rsConstraintValue    sql.NullString
+			rolloutId          string
+			evaluationRollout  storage.EvaluationRollout
+			rtPercentageNumber sql.NullFloat64
+			rtPercentageValue  sql.NullBool
+			rsSegmentKey       sql.NullString
+			rsSegmentValue     sql.NullBool
+			rsMatchType        sql.NullInt32
+			optionalConstraint optionalConstraint
 		)
 
 		if err := rows.Scan(
@@ -242,10 +239,10 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey
 			&rsSegmentKey,
 			&rsSegmentValue,
 			&rsMatchType,
-			&rsConstraintType,
-			&rsConstraintProperty,
-			&rsConstraintOperator,
-			&rsConstraintValue,
+			&optionalConstraint.Type,
+			&optionalConstraint.Property,
+			&optionalConstraint.Operator,
+			&optionalConstraint.Value,
 		); err != nil {
 			return rollouts, err
 		}
@@ -259,19 +256,22 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey
 			evaluationRollout.Threshold = storageThreshold
 		} else if rsSegmentKey.Valid &&
 			rsSegmentValue.Valid &&
-			rsMatchType.Valid &&
-			rsConstraintType.Valid &&
-			rsConstraintProperty.Valid &&
-			rsConstraintOperator.Valid && rsConstraintValue.Valid {
-			c := storage.EvaluationConstraint{
-				Type:     flipt.ComparisonType(rsConstraintType.Int32),
-				Property: rsConstraintProperty.String,
-				Operator: rsConstraintOperator.String,
-				Value:    rsConstraintValue.String,
+			rsMatchType.Valid {
+
+			var c *storage.EvaluationConstraint
+			if optionalConstraint.Type.Valid {
+				c = &storage.EvaluationConstraint{
+					Type:     flipt.ComparisonType(optionalConstraint.Type.Int32),
+					Property: optionalConstraint.Property.String,
+					Operator: optionalConstraint.Operator.String,
+					Value:    optionalConstraint.Value.String,
+				}
 			}
 
 			if existingSegment, ok := uniqueSegmentedRollouts[rolloutId]; ok {
-				existingSegment.Segment.Constraints = append(existingSegment.Segment.Constraints, c)
+				if c != nil {
+					existingSegment.Segment.Constraints = append(existingSegment.Segment.Constraints, *c)
+				}
 				continue
 			}
 
@@ -281,7 +281,9 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey
 				MatchType: flipt.MatchType(rsMatchType.Int32),
 			}
 
-			storageSegment.Constraints = append(storageSegment.Constraints, c)
+			if c != nil {
+				storageSegment.Constraints = append(storageSegment.Constraints, *c)
+			}
 
 			evaluationRollout.Segment = storageSegment
 			uniqueSegmentedRollouts[rolloutId] = &evaluationRollout

internal/storage/sql/evaluation_test.go

@@ -639,16 +639,6 @@ func (s *DBTestSuite) TestGetEvaluationRollouts() {
 
 	require.NoError(t, err)
 
-	_, err = s.store.CreateConstraint(context.TODO(), &flipt.CreateConstraintRequest{
-		SegmentKey: segment.Key,
-		Type:       flipt.ComparisonType_STRING_COMPARISON_TYPE,
-		Property:   "foo",
-		Operator:   "EQ",
-		Value:      "bar",
-	})
-
-	require.NoError(t, err)
-
 	_, err = s.store.CreateRollout(context.TODO(), &flipt.CreateRolloutRequest{
 		FlagKey: flag.Key,
 		Rank:    1,