Merge pull request #43 from checkr/zz/evaluation-return-variant-key

Add variant key validation and put it in the evaluation result

Dockerfile

@@ -9,6 +9,7 @@ ADD ./buildscripts/demo_sqlite3.db /data/demo_sqlite3.db
 ENV FLAGR_DB_DBDRIVER=sqlite3
 ENV FLAGR_DB_DBCONNECTIONSTR=/data/demo_sqlite3.db
 ENV FLAGR_RECORDER_ENABLED=false
+ENV HOST=0.0.0.0
 
 RUN cd ./browser/flagr-ui/ && yarn install && yarn run build
 RUN make build

Makefile

@@ -19,7 +19,7 @@ build: api_docs
 	@CGO_ENABLED=1 go build -o $(PWD)/flagr github.com/checkr/flagr/swagger_gen/cmd/flagr-server
 
 run:
-	@$(PWD)/flagr --port 18000 --host 0.0.0.0
+	@$(PWD)/flagr --port 18000
 
 gen: swagger api_docs goqueryset
 

pkg/entity/flag.go

@@ -17,9 +17,16 @@ type Flag struct {
 	CreatedBy   string
 	UpdatedBy   string
 	Enabled     bool
+	Segments    []Segment
+	Variants    []Variant
 
-	Segments []Segment
-	Variants []Variant
+	// Purely for evaluation
+	FlagEvaluation FlagEvaluation `gorm:"-"`
+}
+
+// FlagEvaluation is a struct that holds the necessary info for evaluation
+type FlagEvaluation struct {
+	VariantsMap map[uint]*Variant
 }
 
 // Preload preloads the segments and variants into flags
@@ -47,26 +54,19 @@ func (f *Flag) Preload(db *gorm.DB) error {
 	return nil
 }
 
-// GetVariant returns the variant in flag
-func (f *Flag) GetVariant(variantID uint) *Variant {
-	if variantID == uint(0) {
-		return nil
-	}
-	for i := range f.Variants {
-		if f.Variants[i].ID == variantID {
-			return &f.Variants[i]
-		}
-	}
-	return nil
-}
-
 // PrepareEvaluation prepares the information for evaluation
 func (f *Flag) PrepareEvaluation() error {
+	f.FlagEvaluation = FlagEvaluation{
+		VariantsMap: make(map[uint]*Variant),
+	}
 	for i := range f.Segments {
 		if err := f.Segments[i].PrepareEvaluation(); err != nil {
 			return err
 		}
 	}
+	for i := range f.Variants {
+		f.FlagEvaluation.VariantsMap[f.Variants[i].ID] = &f.Variants[i]
+	}
 	return nil
 }
 

pkg/entity/segment.go

@@ -11,15 +11,14 @@ import (
 // gen:qs
 type Segment struct {
 	gorm.Model
-
 	FlagID         uint   `gorm:"index:idx_segment_flagid"`
 	Description    string `sql:"type:text"`
 	Rank           uint
 	RolloutPercent uint
+	Constraints    ConstraintArray
+	Distributions  []Distribution
 
-	Constraints   ConstraintArray
-	Distributions []Distribution
-
+	// Purely for evaluation
 	SegmentEvaluation SegmentEvaluation `gorm:"-"`
 }
 

pkg/entity/variant.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/checkr/flagr/pkg/util"
 	"github.com/jinzhu/gorm"
 )
 
@@ -19,6 +20,15 @@ type Variant struct {
 	Attachment Attachment `sql:"type:text"`
 }
 
+// Validate validates the Variant
+func (v *Variant) Validate() error {
+	ok, msg := util.IsSafeKey(v.Key)
+	if !ok {
+		return fmt.Errorf(msg)
+	}
+	return nil
+}
+
 // Attachment supports dynamic configuration in variant
 type Attachment map[string]string
 

pkg/handler/crud.go

@@ -358,8 +358,11 @@ func (c *crud) CreateVariant(params variant.CreateVariantParams) middleware.Resp
 	}
 	v.Attachment = a
 
-	err = v.Create(repo.GetDB())
-	if err != nil {
+	if err := v.Validate(); err != nil {
+		return variant.NewCreateVariantDefault(400).WithPayload(ErrorMessage("%s", err))
+	}
+
+	if err := v.Create(repo.GetDB()); err != nil {
 		return variant.NewCreateVariantDefault(500).WithPayload(ErrorMessage("%s", err))
 	}
 
@@ -400,6 +403,10 @@ func (c *crud) PutVariant(params variant.PutVariantParams) middleware.Responder
 		v.Attachment = a
 	}
 
+	if err := v.Validate(); err != nil {
+		return variant.NewPutVariantDefault(400).WithPayload(ErrorMessage("%s", err))
+	}
+
 	if err := repo.GetDB().Save(&v).Error; err != nil {
 		return variant.NewPutVariantDefault(500).WithPayload(ErrorMessage("%s", err))
 	}

pkg/handler/eval.go

@@ -100,9 +100,10 @@ func evalFlag(evalContext *models.EvalContext) (*models.EvalResult, *Error) {
 	evalResult.EvalDebugLog.SegmentDebugLogs = logs
 	evalResult.SegmentID = sID
 	evalResult.VariantID = vID
-	v := f.GetVariant(util.SafeUint(vID))
+	v := f.FlagEvaluation.VariantsMap[util.SafeUint(vID)]
 	if v != nil {
 		evalResult.VariantAttachment = v.Attachment
+		evalResult.VariantKey = util.StringPtr(v.Key)
 	}
 
 	asyncRecord(evalResult)

pkg/util/util.go

@@ -1,9 +1,24 @@
 package util
 
 import (
+	"fmt"
+	"regexp"
 	"time"
 )
 
+var keyRegex = regexp.MustCompile("^[a-z]+[a-z0-9_]*$")
+
+// IsSafeKey return if the key is safe to store
+func IsSafeKey(s string) (bool, string) {
+	if !keyRegex.MatchString(s) {
+		return false, fmt.Sprintf("key should have the format %v", keyRegex)
+	}
+	if len(s) > 63 {
+		return false, fmt.Sprintf("the key is too long")
+	}
+	return true, ""
+}
+
 // SafeString safely dereference the string or string ptr
 func SafeString(s interface{}) (r string) {
 	defer func() {

pkg/util/util_test.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -29,3 +30,36 @@ func TestSafeUint(t *testing.T) {
 func TestTimeNow(t *testing.T) {
 	assert.Len(t, TimeNow(), 20)
 }
+
+func TestIsSafeKey(t *testing.T) {
+	var b bool
+	var msg string
+
+	b, msg = IsSafeKey("a")
+	assert.True(t, b)
+	assert.Empty(t, msg)
+
+	b, msg = IsSafeKey("a1")
+	assert.True(t, b)
+	assert.Empty(t, msg)
+
+	b, msg = IsSafeKey("a_1")
+	assert.True(t, b)
+	assert.Empty(t, msg)
+
+	b, msg = IsSafeKey(strings.Repeat("a", 63))
+	assert.True(t, b)
+	assert.Empty(t, msg)
+
+	b, msg = IsSafeKey("1a")
+	assert.False(t, b)
+	assert.NotEmpty(t, msg)
+
+	b, msg = IsSafeKey("_a")
+	assert.False(t, b)
+	assert.NotEmpty(t, msg)
+
+	b, msg = IsSafeKey(strings.Repeat("a", 64))
+	assert.False(t, b)
+	assert.NotEmpty(t, msg)
+}

swagger/index.yaml

@@ -326,6 +326,8 @@ definitions:
       - flagID
       - segmentID
       - variantID
+      - variantKey
+      - variantAttachment
       - evalContext
       - timestamp
     properties:
@@ -341,15 +343,18 @@ definitions:
         type: integer
         format: int64
         minimum: 1
+      variantKey:
+        type: string
+        minLength: 1
+      variantAttachment:
+        type: object
       evalContext:
         $ref: "#/definitions/evalContext"
       timestamp:
         type: string
         minLength: 1
       evalDebugLog:
         $ref: "#/definitions/evalDebugLog"
-      variantAttachment:
-        type: object
   evalDebugLog:
     type: object
     properties: