Skip to content

Commit

Permalink
feat: default variant redo
Browse files Browse the repository at this point in the history 
Signed-off-by: Mark Phelps <[email protected]>
  • Loading branch information
@markphelps
markphelps committed Jul 12, 2024
1 parent 3e72b57 commit 9e88dc5
Show file tree
Hide file tree
Showing 7 changed files with 267 additions and 188 deletions.
1 change: 1 addition & 0 deletions config/migrations/sqlite3/13_default_variant.up.sql
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE `flags` ADD COLUMN `default_variant_id` VARCHAR(255) REFERENCES variants(`id`) ON DELETE SET NULL;
5 changes: 5 additions & 0 deletions go.work.sum
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
cel.dev/expr v0.15.0/go.mod h1:TRSuuV7DlVCE/uwv5QbAiW/v8l5O8C4eEPHeu7gf7Sg=
cloud.google.com/go v0.112.1/go.mod h1:+Vbu+Y1UU+I1rjmzeMOb/8RfkKJK2Gyxi1X6jJCZLo4=
cloud.google.com/go/accessapproval v1.7.6/go.mod h1:bdDCS3iLSLhlK3pu8lJClaeIVghSpTLGChl1Ihr9Fsc=
cloud.google.com/go/accesscontextmanager v1.8.6/go.mod h1:rMC0Z8pCe/JR6yQSksprDc6swNKjMEvkfCbaesh+OS0=
Expand Down Expand Up @@ -200,6 +201,7 @@ github.com/circonus-labs/circonusllhist v0.1.3/go.mod h1:kMXHVDlOchFAehlya5ePtbp
github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk=
github.com/cncf/udpa/go v0.0.0-20220112060539-c52dc94e7fbe/go.mod h1:6pvJx4me5XPnfI9Z40ddWsdw2W/uZgQLFXToKeRcDiI=
github.com/cncf/xds/go v0.0.0-20240318125728-8a4994d93e50/go.mod h1:5e1+Vvlzido69INQaVO6d87Qn543Xr6nooe9Kz7oBFM=
github.com/cncf/xds/go v0.0.0-20240423153145-555b57ec207b/go.mod h1:W+zGtBO5Y1IgJhy4+A9GOqVhqLpfZi+vwmdNXUehLA8=
github.com/cockroachdb/apd v1.1.0 h1:3LFP3629v+1aKXU5Q37mxmRxX/pIu1nijXydLShEq5I=
github.com/containerd/aufs v1.0.0/go.mod h1:kL5kd6KM5TzQjR79jljyi4olc1Vrx6XBlcyj3gNv2PU=
github.com/containerd/btrfs/v2 v2.0.0/go.mod h1:swkD/7j9HApWpzl8OHfrHNxppPd9l44DFZdF94BUj9k=
Expand Down Expand Up @@ -517,6 +519,7 @@ go.etcd.io/etcd/client/v3 v3.5.12/go.mod h1:tSbBCakoWmmddL+BKVAJHa9km+O/E+bumDe9
go.flipt.io/stew v0.0.0-20240109140408-33ad11ecef1c/go.mod h1:uCrn9WPz5WKYJ7+YXQ/mFO7vIp06PxkxHnPzOpcgMD0=
go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1/go.mod h1:SNgMg+EgDFwmvSmLRTNKC5fegJjB7v23qTQ0XLGUNHk=
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0/go.mod h1:Mjt1i1INqiaoZOMGR1RIUJN+i3ChKoFRqzrRQhlkbs0=
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.51.0/go.mod h1:27iA5uvhuRNmalO+iEUdVn5ZMj2qy10Mm+XRIpRmyuU=
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0/go.mod h1:p8pYQP+m5XfbZm9fxtSKAbM6oIllS7s2AfxrChvc7iw=
go.opentelemetry.io/otel v1.24.0/go.mod h1:W7b9Ozg4nkF5tWI5zsXkaKKDjdVjpD4oAt9Qi/MArHo=
go.opentelemetry.io/otel v1.26.0/go.mod h1:UmLkJHUAidDval2EICqBMbnAd0/m2vmpf/dAM+fvFs4=
Expand Down Expand Up @@ -587,6 +590,7 @@ google.golang.org/genproto/googleapis/api v0.0.0-20240325203815-454cdb8f5daa/go.
google.golang.org/genproto/googleapis/api v0.0.0-20240415180920-8c6c420018be/go.mod h1:dvdCTIoAGbkWbcIKBniID56/7XHTt6WfxXNMxuziJ+w=
google.golang.org/genproto/googleapis/api v0.0.0-20240429193739-8cf5692501f6/go.mod h1:10yRODfgim2/T8csjQsMPgZOMvtytXKTDRzH6HRGzRw=
google.golang.org/genproto/googleapis/api v0.0.0-20240506185236-b8a5c65736ae/go.mod h1:FfiGhwUm6CJviekPrc0oJ+7h29e+DmWU6UtjX0ZvI7Y=
google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157/go.mod h1:99sLkeliLXfdj2J75X3Ho+rrVCaJze0uwN7zDDkjPVU=
google.golang.org/genproto/googleapis/bytestream v0.0.0-20240429193739-8cf5692501f6/go.mod h1:ULqtoQMxDLNRfW+pJbKA68wtIy1OiYjdIsJs3PMpzh8=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240227224415-6ceb2ff114de/go.mod h1:H4O17MA/PE9BsGx3w+a+W2VOLLD1Qf7oJneAoU6WktY=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240304161311-37d4d3c04a78/go.mod h1:UCOku4NytXMJuLQE5VuqA5lX3PcHCBo8pxNyvkf4xBs=
Expand All @@ -595,6 +599,7 @@ google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237/go.
google.golang.org/genproto/googleapis/rpc v0.0.0-20240325203815-454cdb8f5daa/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240415180920-8c6c420018be/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240429193739-8cf5692501f6/go.mod h1:WtryC6hu0hhx87FDGxWCDptyssuo68sk10vYjF+T9fY=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240528184218-531527333157/go.mod h1:EfXuqaE1J41VCDicxHzUDm+8rk+7ZdXzHV0IhO/I6s0=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240610135401-a8a62080eff3/go.mod h1:EfXuqaE1J41VCDicxHzUDm+8rk+7ZdXzHV0IhO/I6s0=
google.golang.org/grpc v1.37.0/go.mod h1:NREThFqKR1f3iQ6oBuvc5LadQuXVGo9rkm5ZGrQdJfM=
google.golang.org/grpc v1.62.1/go.mod h1:IWTG0VlJLCh1SkC58F7np9ka9mx/WNkjl4PGJaiq+QE=
Expand Down
20 changes: 19 additions & 1 deletion internal/server/evaluation/legacy_evaluator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ func (e *Evaluator) Evaluate(ctx context.Context, flag *flipt.Flag, r *evaluatio
if !flag.Enabled {
resp.Match = false
resp.Reason = flipt.EvaluationReason_FLAG_DISABLED_EVALUATION_REASON

if flag.DefaultVariant != nil {
resp.Value = flag.DefaultVariant.Key
resp.Attachment = flag.DefaultVariant.Attachment

Check warning on line 100 in internal/server/evaluation/legacy_evaluator.go

View check run for this annotation

Codecov / codecov/patch

internal/server/evaluation/legacy_evaluator.go#L99-L100 

Added lines #L99 - L100 were not covered by tests
}

return resp, nil
}

Expand All @@ -105,6 +111,10 @@ func (e *Evaluator) Evaluate(ctx context.Context, flag *flipt.Flag, r *evaluatio

if len(rules) == 0 {
e.logger.Debug("no rules match")
if flag.DefaultVariant != nil {
resp.Value = flag.DefaultVariant.Key
resp.Attachment = flag.DefaultVariant.Attachment

Check warning on line 116 in internal/server/evaluation/legacy_evaluator.go

View check run for this annotation

Codecov / codecov/patch

internal/server/evaluation/legacy_evaluator.go#L115-L116 

Added lines #L115 - L116 were not covered by tests
}
return resp, nil
}

Expand Down Expand Up @@ -189,6 +199,10 @@ func (e *Evaluator) Evaluate(ctx context.Context, flag *flipt.Flag, r *evaluatio
e.logger.Info("no distributions for rule")
resp.Match = true
resp.Reason = flipt.EvaluationReason_MATCH_EVALUATION_REASON
if flag.DefaultVariant != nil {
resp.Value = flag.DefaultVariant.Key
resp.Attachment = flag.DefaultVariant.Attachment

Check warning on line 204 in internal/server/evaluation/legacy_evaluator.go

View check run for this annotation

Codecov / codecov/patch

internal/server/evaluation/legacy_evaluator.go#L203-L204 

Added lines #L203 - L204 were not covered by tests
}
return resp, nil
}

Expand All @@ -202,8 +216,12 @@ func (e *Evaluator) Evaluate(ctx context.Context, flag *flipt.Flag, r *evaluatio

// if index is outside of our existing buckets then it does not match any distribution
if index == len(validDistributions) {
resp.Match = false
e.logger.Debug("did not match any distributions")
resp.Match = false
if flag.DefaultVariant != nil {
resp.Value = flag.DefaultVariant.Key
resp.Attachment = flag.DefaultVariant.Attachment

Check warning on line 223 in internal/server/evaluation/legacy_evaluator.go

View check run for this annotation

Codecov / codecov/patch

internal/server/evaluation/legacy_evaluator.go#L222-L223 

Added lines #L222 - L223 were not covered by tests
}
return resp, nil
}

Expand Down
55 changes: 41 additions & 14 deletions internal/storage/sql/common/flag.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ func emptyAsNil(str string) *string {
// GetFlag gets a flag with variants by key
func (s *Store) GetFlag(ctx context.Context, p storage.ResourceRequest) (*flipt.Flag, error) {
var (
createdAt fliptsql.Timestamp
updatedAt fliptsql.Timestamp
createdAt fliptsql.Timestamp
updatedAt fliptsql.Timestamp
defaultVariantId sql.NullString

flag = &flipt.Flag{}

err = s.builder.Select("namespace_key, \"key\", \"type\", name, description, enabled, created_at, updated_at").
err = s.builder.Select("namespace_key, \"key\", \"type\", name, description, enabled, created_at, updated_at, default_variant_id").
From("flags").
Where(sq.Eq{"namespace_key": p.Namespace(), "\"key\"": p.Key}).
QueryRowContext(ctx).
Expand All @@ -53,7 +54,8 @@ func (s *Store) GetFlag(ctx context.Context, p storage.ResourceRequest) (*flipt.
&flag.Description,
&flag.Enabled,
&createdAt,
&updatedAt)
&updatedAt,
&defaultVariantId)
)

if err != nil {
Expand Down Expand Up @@ -105,6 +107,7 @@ func (s *Store) GetFlag(ctx context.Context, p storage.ResourceRequest) (*flipt.

variant.CreatedAt = createdAt.Timestamp
variant.UpdatedAt = updatedAt.Timestamp

if attachment.Valid {
compactedAttachment, err := compactJSONString(attachment.String)
if err != nil {
Expand All @@ -113,6 +116,10 @@ func (s *Store) GetFlag(ctx context.Context, p storage.ResourceRequest) (*flipt.
variant.Attachment = compactedAttachment
}

if defaultVariantId.Valid && variant.Id == defaultVariantId.String {
flag.DefaultVariant = &variant

Check warning on line 120 in internal/storage/sql/common/flag.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/sql/common/flag.go#L120 

Added line #L120 was not covered by tests
}

flag.Variants = append(flag.Variants, &variant)
}

Expand All @@ -131,13 +138,18 @@ type optionalVariant struct {
UpdatedAt fliptsql.NullableTimestamp
}

type flagWithDefaultVariant struct {
*flipt.Flag
DefaultVariantId sql.NullString
}

// ListFlags lists all flags with variants
func (s *Store) ListFlags(ctx context.Context, req *storage.ListRequest[storage.NamespaceRequest]) (storage.ResultSet[*flipt.Flag], error) {
var (
flags []*flipt.Flag
results = storage.ResultSet[*flipt.Flag]{}

query = s.builder.Select("namespace_key, \"key\", \"type\", name, description, enabled, created_at, updated_at").
query = s.builder.Select("namespace_key, \"key\", \"type\", name, description, enabled, created_at, updated_at, default_variant_id").
From("flags").
Where(sq.Eq{"namespace_key": req.Predicate.Namespace()}).
OrderBy(fmt.Sprintf("created_at %s", req.QueryParams.Order))
Expand Down Expand Up @@ -174,13 +186,14 @@ func (s *Store) ListFlags(ctx context.Context, req *storage.ListRequest[storage.
}()

// keep track of flags so we can associated variants in second query.
flagsByKey := make(map[string]*flipt.Flag)
flagsByKey := make(map[string]*flagWithDefaultVariant)
for rows.Next() {
var (
flag = &flipt.Flag{}

fCreatedAt fliptsql.Timestamp
fUpdatedAt fliptsql.Timestamp
fCreatedAt fliptsql.Timestamp
fUpdatedAt fliptsql.Timestamp
fDefaultVariantId sql.NullString
)

if err := rows.Scan(
Expand All @@ -191,15 +204,17 @@ func (s *Store) ListFlags(ctx context.Context, req *storage.ListRequest[storage.
&flag.Description,
&flag.Enabled,
&fCreatedAt,
&fUpdatedAt); err != nil {
&fUpdatedAt,
&fDefaultVariantId,
); err != nil {
return results, err
}

flag.CreatedAt = fCreatedAt.Timestamp
flag.UpdatedAt = fUpdatedAt.Timestamp

flags = append(flags, flag)
flagsByKey[flag.Key] = flag
flagsByKey[flag.Key] = &flagWithDefaultVariant{Flag: flag, DefaultVariantId: fDefaultVariantId}
}

if err := rows.Err(); err != nil {
Expand Down Expand Up @@ -234,7 +249,7 @@ func (s *Store) ListFlags(ctx context.Context, req *storage.ListRequest[storage.
return results, nil
}

func (s *Store) setVariants(ctx context.Context, namespaceKey string, flagsByKey map[string]*flipt.Flag) error {
func (s *Store) setVariants(ctx context.Context, namespaceKey string, flagsByKey map[string]*flagWithDefaultVariant) error {
allFlagKeys := make([]string, 0, len(flagsByKey))
for k := range flagsByKey {
allFlagKeys = append(allFlagKeys, k)
Expand Down Expand Up @@ -277,7 +292,7 @@ func (s *Store) setVariants(ctx context.Context, namespaceKey string, flagsByKey
}

if flag, ok := flagsByKey[variant.FlagKey.String]; ok {
flag.Variants = append(flag.Variants, &flipt.Variant{
v := &flipt.Variant{
Id: variant.Id.String,
NamespaceKey: variant.NamespaceKey.String,
Key: variant.Key.String,
Expand All @@ -287,7 +302,13 @@ func (s *Store) setVariants(ctx context.Context, namespaceKey string, flagsByKey
Attachment: variant.Attachment.String,
CreatedAt: vCreatedAt.Timestamp,
UpdatedAt: vUpdatedAt.Timestamp,
})
}

flag.Variants = append(flag.Variants, v)

if flag.DefaultVariantId.Valid && variant.Id.String == flag.DefaultVariantId.String {
flag.DefaultVariant = v

Check warning on line 310 in internal/storage/sql/common/flag.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/sql/common/flag.go#L310 

Added line #L310 was not covered by tests
}
}
}

Expand Down Expand Up @@ -374,7 +395,13 @@ func (s *Store) UpdateFlag(ctx context.Context, r *flipt.UpdateFlagRequest) (_ *
Set("name", r.Name).
Set("description", r.Description).
Set("enabled", r.Enabled).
Set("updated_at", &fliptsql.Timestamp{Timestamp: flipt.Now()}).
Set("updated_at", &fliptsql.Timestamp{Timestamp: flipt.Now()})

if r.DefaultVariantId != "" {
query = query.Set("default_variant_id", r.DefaultVariantId)

Check warning on line 401 in internal/storage/sql/common/flag.go

View check run for this annotation

Codecov / codecov/patch

internal/storage/sql/common/flag.go#L401 

Added line #L401 was not covered by tests
}

query = query.
Where(sq.And{sq.Eq{"namespace_key": r.NamespaceKey}, sq.Eq{"\"key\"": r.Key}})

res, err := query.ExecContext(ctx)
Expand Down
4 changes: 2 additions & 2 deletions internal/storage/sql/migrator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import (
)

var expectedVersions = map[Driver]uint{
SQLite: 12,
LibSQL: 12, // libsql driver uses the same migrations as sqlite3
SQLite: 13,
LibSQL: 13, // libsql driver uses the same migrations as sqlite3
Postgres: 13,
MySQL: 12,
CockroachDB: 10,
Expand Down
Loading

0 comments on commit 9e88dc5

Please sign in to comment.