Merge pull request #118 from checkr/zz/make-entity-id-nil-error

Use random entity id if it's nil
openflagr · May 1, 2018 · c4bb880 · c4bb880
2 parents 1ddb473 + 269bbca
commit c4bb880
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 84 deletions.
diff --git a/docs/api_docs/bundle.yaml b/docs/api_docs/bundle.yaml
@@ -968,13 +968,14 @@ definitions:
   evalContext:
     type: object
     required:
-      - entityID
       - entityType
       - flagID
     properties:
       entityID:
         type: string
-        minLength: 1
+        description: >-
+          entityID is used to deterministically at random to evaluate the flag
+          result. If it's empty, flagr will randomly generate one.
       entityType:
         type: string
         minLength: 1
@@ -1045,12 +1046,10 @@ definitions:
   evaluationEntity:
     type: object
     required:
-      - entityID
       - entityType
     properties:
       entityID:
         type: string
-        minLength: 1
       entityType:
         type: string
         minLength: 1

diff --git a/pkg/entity/distribution.go b/pkg/entity/distribution.go
@@ -46,8 +46,8 @@ type DistributionDebugLog struct {
 }
 
 // Rollout rolls out the entity based on the rolloutPercent
-func (d DistributionArray) Rollout(entityID *string, salt string, rolloutPercent uint) (variantID *uint, msg string) {
-	if entityID == nil || *entityID == "" {
+func (d DistributionArray) Rollout(entityID string, salt string, rolloutPercent uint) (variantID *uint, msg string) {
+	if entityID == "" {
 		return nil, "rollout no. empty entityID"
 	}
 
@@ -59,7 +59,7 @@ func (d DistributionArray) Rollout(entityID *string, salt string, rolloutPercent
 		return nil, "rollout no. there's no distribution set"
 	}
 
-	num := crc32Num(*entityID, salt)
+	num := crc32Num(entityID, salt)
 	vID, index := d.bucketByNum(num)
 	log := fmt.Sprintf("%+v", DistributionDebugLog{
 		BucketNum:         num,

diff --git a/pkg/entity/distribution_test.go b/pkg/entity/distribution_test.go
@@ -3,7 +3,6 @@ package entity
 import (
 	"testing"
 
-	"github.com/checkr/flagr/pkg/util"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -148,19 +147,19 @@ func TestRolloutWithEntity(t *testing.T) {
 		var vID *uint
 		var msg string
 
-		vID, msg = d.Rollout(nil, "salt", uint(0))
+		vID, msg = d.Rollout("", "salt", uint(0))
 		assert.Nil(t, vID)
 		assert.Contains(t, msg, "no")
 
-		vID, msg = d.Rollout(util.StringPtr("entity123"), "salt", uint(0))
+		vID, msg = d.Rollout("entity123", "salt", uint(0))
 		assert.Nil(t, vID)
 		assert.Contains(t, msg, "no")
 
-		vID, msg = d.Rollout(util.StringPtr("entity123"), "salt", uint(100))
+		vID, msg = d.Rollout("entity123", "salt", uint(100))
 		assert.NotNil(t, vID)
 		assert.Contains(t, msg, "yes")
 
-		vID, msg = d.Rollout(util.StringPtr("entity123"), "salt", uint(1))
+		vID, msg = d.Rollout("entity123", "salt", uint(1))
 		assert.Nil(t, vID)
 		assert.Contains(t, msg, "no")
 	})
@@ -173,7 +172,7 @@ func TestRolloutWithEntity(t *testing.T) {
 		var vID *uint
 		var msg string
 
-		vID, msg = d.Rollout(util.StringPtr("entity123"), "salt", uint(100))
+		vID, msg = d.Rollout("entity123", "salt", uint(100))
 		assert.Nil(t, vID)
 		assert.Contains(t, msg, "no")
 	})

diff --git a/pkg/handler/data_recorder_kafka_test.go b/pkg/handler/data_recorder_kafka_test.go
@@ -83,7 +83,7 @@ func TestKafkaEvalResult(t *testing.T) {
 		r := &kafkaEvalResult{
 			EvalResult: &models.EvalResult{
 				EvalContext: &models.EvalContext{
-					EntityID: util.StringPtr("123"),
+					EntityID: "123",
 				},
 				FlagID:         util.Int64Ptr(int64(1)),
 				FlagSnapshotID: 1,

diff --git a/pkg/handler/eval.go b/pkg/handler/eval.go
@@ -3,6 +3,7 @@ package handler
 import (
 	"encoding/json"
 	"fmt"
+	"math/rand"
 	"time"
 
 	"github.com/checkr/flagr/pkg/config"
@@ -106,6 +107,10 @@ var evalFlag = func(evalContext models.EvalContext) *models.EvalResult {
 		return BlankResult(f, evalContext, fmt.Sprintf("flagID %v has no segments", flagID))
 	}
 
+	if evalContext.EntityID == "" {
+		evalContext.EntityID = fmt.Sprintf("randomly_generated_%d", rand.Int31())
+	}
+
 	logs := []*models.SegmentDebugLog{}
 	var vID *int64
 	var sID *int64

diff --git a/pkg/handler/eval_test.go b/pkg/handler/eval_test.go
@@ -28,7 +28,7 @@ func TestEvalSegment(t *testing.T) {
 		vID, log := evalSegment(models.EvalContext{
 			EnableDebug:   true,
 			EntityContext: map[string]interface{}{"dl_state": "CA"},
-			EntityID:      util.StringPtr("entityID1"),
+			EntityID:      "entityID1",
 			EntityType:    util.StringPtr("entityType1"),
 			FlagID:        util.Int64Ptr(int64(100)),
 		}, s)
@@ -43,7 +43,7 @@ func TestEvalSegment(t *testing.T) {
 		vID, log := evalSegment(models.EvalContext{
 			EnableDebug:   true,
 			EntityContext: map[string]interface{}{},
-			EntityID:      util.StringPtr("entityID1"),
+			EntityID:      "entityID1",
 			EntityType:    util.StringPtr("entityType1"),
 			FlagID:        util.Int64Ptr(int64(100)),
 		}, s)
@@ -58,7 +58,7 @@ func TestEvalSegment(t *testing.T) {
 		vID, log := evalSegment(models.EvalContext{
 			EnableDebug:   true,
 			EntityContext: map[string]interface{}{"dl_state": "NY"},
-			EntityID:      util.StringPtr("entityID1"),
+			EntityID:      "entityID1",
 			EntityType:    util.StringPtr("entityType1"),
 			FlagID:        util.Int64Ptr(int64(100)),
 		}, s)
@@ -73,7 +73,7 @@ func TestEvalSegment(t *testing.T) {
 		vID, log := evalSegment(models.EvalContext{
 			EnableDebug:   true,
 			EntityContext: nil,
-			EntityID:      util.StringPtr("entityID1"),
+			EntityID:      "entityID1",
 			EntityType:    util.StringPtr("entityType1"),
 			FlagID:        util.Int64Ptr(int64(100)),
 		}, s)
@@ -88,16 +88,17 @@ func TestEvalFlag(t *testing.T) {
 
 	t.Run("test empty evalContext", func(t *testing.T) {
 		defer gostub.StubFunc(&GetEvalCache, GenFixtureEvalCache()).Reset()
-		result := evalFlag(models.EvalContext{})
+		result := evalFlag(models.EvalContext{FlagID: util.Int64Ptr(int64(100))})
 		assert.Nil(t, result.VariantID)
+		assert.NotEmpty(t, result.EvalContext.EntityID)
 	})
 
 	t.Run("test happy code path", func(t *testing.T) {
 		defer gostub.StubFunc(&GetEvalCache, GenFixtureEvalCache()).Reset()
 		result := evalFlag(models.EvalContext{
 			EnableDebug:   true,
 			EntityContext: map[string]interface{}{"dl_state": "CA"},
-			EntityID:      util.StringPtr("entityID1"),
+			EntityID:      "entityID1",
 			EntityType:    util.StringPtr("entityType1"),
 			FlagID:        util.Int64Ptr(int64(100)),
 		})
@@ -136,7 +137,7 @@ func TestEvalFlag(t *testing.T) {
 		result := evalFlag(models.EvalContext{
 			EnableDebug:   true,
 			EntityContext: map[string]interface{}{"dl_state": "CA", "state": "CA", "rate": 2000},
-			EntityID:      util.StringPtr("entityID1"),
+			EntityID:      "entityID1",
 			EntityType:    util.StringPtr("entityType1"),
 			FlagID:        util.Int64Ptr(int64(100)),
 		})
@@ -168,7 +169,7 @@ func TestEvalFlag(t *testing.T) {
 		result := evalFlag(models.EvalContext{
 			EnableDebug:   true,
 			EntityContext: map[string]interface{}{"dl_state": "CA", "state": "NY"},
-			EntityID:      util.StringPtr("entityID1"),
+			EntityID:      "entityID1",
 			EntityType:    util.StringPtr("entityType1"),
 			FlagID:        util.Int64Ptr(int64(100)),
 		})
@@ -184,7 +185,7 @@ func TestEvalFlag(t *testing.T) {
 		result := evalFlag(models.EvalContext{
 			EnableDebug:   true,
 			EntityContext: map[string]interface{}{"dl_state": "CA"},
-			EntityID:      util.StringPtr("entityID1"),
+			EntityID:      "entityID1",
 			EntityType:    util.StringPtr("entityType1"),
 			FlagID:        util.Int64Ptr(int64(100)),
 		})
@@ -208,7 +209,7 @@ func TestPostEvaluation(t *testing.T) {
 			Body: &models.EvalContext{
 				EnableDebug:   true,
 				EntityContext: map[string]interface{}{"dl_state": "CA", "state": "NY"},
-				EntityID:      util.StringPtr("entityID1"),
+				EntityID:      "entityID1",
 				EntityType:    util.StringPtr("entityType1"),
 				FlagID:        util.Int64Ptr(int64(100)),
 			},
@@ -227,7 +228,7 @@ func TestPostEvaluationBatch(t *testing.T) {
 				Entities: []*models.EvaluationEntity{
 					{
 						EntityContext: map[string]interface{}{"dl_state": "CA", "state": "NY"},
-						EntityID:      util.StringPtr("entityID1"),
+						EntityID:      "entityID1",
 						EntityType:    util.StringPtr("entityType1"),
 					},
 				},

diff --git a/swagger/index.yaml b/swagger/index.yaml
@@ -348,13 +348,12 @@ definitions:
   evalContext:
     type: object
     required:
-      - entityID
       - entityType
       - flagID
     properties:
       entityID:
         type: string
-        minLength: 1
+        description: entityID is used to deterministically at random to evaluate the flag result. If it's empty, flagr will randomly generate one.
       entityType:
         type: string
         minLength: 1
@@ -427,12 +426,10 @@ definitions:
   evaluationEntity:
     type: object
     required:
-      - entityID
       - entityType
     properties:
       entityID:
         type: string
-        minLength: 1
       entityType:
         type: string
         minLength: 1

diff --git a/swagger_gen/models/eval_context.go b/swagger_gen/models/eval_context.go
diff --git a/swagger_gen/models/evaluation_entity.go b/swagger_gen/models/evaluation_entity.go