Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(rollouts): account for segments without constraints #1919

Merged
merged 2 commits into from
Jul 28, 2023
Merged

chore(rollouts): account for segments without constraints #1919

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
879c1bb
chore(rollouts): account for segments without constraints
yquansah Jul 28, 2023
9dfe7c2
chore(production): use same configuration for default
yquansah Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions build/testing/integration/readonly/readonly_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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...)

Expand All @@ -144,7 +144,7 @@ func TestReadOnly(t *testing.T) {
nextPage = flags.NextPageToken
}

require.Len(t, found, 52)
require.Len(t, found, 53)
})
})

Expand Down Expand Up @@ -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 (
Expand All @@ -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)
})
})

Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 13 additions & 0 deletions build/testing/integration/readonly/testdata/default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
56 changes: 29 additions & 27 deletions internal/storage/sql/common/evaluation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}).
Expand All @@ -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(
Expand All @@ -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
}
Expand All @@ -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
}

Expand All @@ -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
Expand Down
10 changes: 0 additions & 10 deletions internal/storage/sql/evaluation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the CreateConstraint here to ensure that the query still picks up the necessary information.


_, err = s.store.CreateRollout(context.TODO(), &flipt.CreateRolloutRequest{
FlagKey: flag.Key,
Rank: 1,
Expand Down