Fix evaluation does not consistently maintain distribution order when…

… using Postgres (#233)

Fix evaluation does not consistently maintain distribution order when using Postgres (#233)

CHANGELOG.md

@@ -3,19 +3,25 @@
 This format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [v0.12.0](https://github.com/markphelps/flipt/releases/tag/v0.12.0) - 2020-02-01
+## [v0.12.1](https://github.com/markphelps/flipt/releases/tag/v0.12.1) - 2020-02-18
 
-### Changed
+### Fixed
 
-* Fixed documentation link in app
-* Underlying caching library from golang-lru to go-cache
+* Issue where distributions did not always maintain order during evaluation when using Postgres [https://github.com/markphelps/flipt/issues/229](https://github.com/markphelps/flipt/issues/229)
+
+## [v0.12.0](https://github.com/markphelps/flipt/releases/tag/v0.12.0) - 2020-02-01
 
 ### Added
 
 * Caching support for segments, rules AND evaluation! [https://github.com/markphelps/flipt/issues/100](https://github.com/markphelps/flipt/issues/100)
 * `cache.memory.expiration` configuration option
 * `cache.memory.eviction_interval` configuration option
 
+### Changed
+
+* Fixed documentation link in app
+* Underlying caching library from golang-lru to go-cache
+
 ### Removed
 
 * `cache.memory.items` configuration option

storage/db/evaluation.go

@@ -116,6 +116,7 @@ func (s *EvaluationStore) GetEvaluationDistributions(ctx context.Context, ruleID
 		From("distributions d").
 		Join("variants v ON (d.variant_id = v.id)").
 		Where(sq.Eq{"d.rule_id": ruleID}).
+		OrderBy("d.created_at").
 		QueryContext(ctx)
 	if err != nil {
 		return nil, err

storage/db/evaluation_test.go

@@ -178,3 +178,136 @@ func TestGetEvaluationDistributions(t *testing.T) {
 	assert.Equal(t, variant2.Key, evaluationDistributions[1].VariantKey)
 	assert.Equal(t, float32(50.00), evaluationDistributions[1].Rollout)
 }
+
+// https://github.com/markphelps/flipt/issues/229
+func TestGetEvaluationDistributions_MaintainOrder(t *testing.T) {
+	flag, err := flagStore.CreateFlag(context.TODO(), &flipt.CreateFlagRequest{
+		Key:         t.Name(),
+		Name:        "foo",
+		Description: "bar",
+		Enabled:     true,
+	})
+
+	require.NoError(t, err)
+
+	// variant 1
+	variant1, err := flagStore.CreateVariant(context.TODO(), &flipt.CreateVariantRequest{
+		FlagKey: flag.Key,
+		Key:     "foo",
+	})
+
+	require.NoError(t, err)
+
+	// variant 2
+	variant2, err := flagStore.CreateVariant(context.TODO(), &flipt.CreateVariantRequest{
+		FlagKey: flag.Key,
+		Key:     "bar",
+	})
+
+	require.NoError(t, err)
+
+	segment, err := segmentStore.CreateSegment(context.TODO(), &flipt.CreateSegmentRequest{
+		Key:         t.Name(),
+		Name:        "foo",
+		Description: "bar",
+		MatchType:   flipt.MatchType_ANY_MATCH_TYPE,
+	})
+
+	require.NoError(t, err)
+
+	_, err = segmentStore.CreateConstraint(context.TODO(), &flipt.CreateConstraintRequest{
+		SegmentKey: segment.Key,
+		Type:       flipt.ComparisonType_STRING_COMPARISON_TYPE,
+		Property:   "foo",
+		Operator:   "EQ",
+		Value:      "bar",
+	})
+
+	require.NoError(t, err)
+
+	rule, err := ruleStore.CreateRule(context.TODO(), &flipt.CreateRuleRequest{
+		FlagKey:    flag.Key,
+		SegmentKey: segment.Key,
+		Rank:       1,
+	})
+
+	require.NoError(t, err)
+
+	// 80/20 distribution
+	dist1, err := ruleStore.CreateDistribution(context.TODO(), &flipt.CreateDistributionRequest{
+		FlagKey:   flag.Key,
+		RuleId:    rule.Id,
+		VariantId: variant1.Id,
+		Rollout:   80.00,
+	})
+
+	require.NoError(t, err)
+
+	dist2, err := ruleStore.CreateDistribution(context.TODO(), &flipt.CreateDistributionRequest{
+		FlagKey:   flag.Key,
+		RuleId:    rule.Id,
+		VariantId: variant2.Id,
+		Rollout:   20.00,
+	})
+
+	require.NoError(t, err)
+
+	evaluationDistributions, err := evaluationStore.GetEvaluationDistributions(context.TODO(), rule.Id)
+	require.NoError(t, err)
+
+	assert.Equal(t, 2, len(evaluationDistributions))
+
+	assert.NotEmpty(t, evaluationDistributions[0].ID)
+	assert.Equal(t, rule.Id, evaluationDistributions[0].RuleID)
+	assert.Equal(t, variant1.Id, evaluationDistributions[0].VariantID)
+	assert.Equal(t, variant1.Key, evaluationDistributions[0].VariantKey)
+	assert.Equal(t, float32(80.00), evaluationDistributions[0].Rollout)
+
+	assert.NotEmpty(t, evaluationDistributions[1].ID)
+	assert.Equal(t, rule.Id, evaluationDistributions[1].RuleID)
+	assert.Equal(t, variant2.Id, evaluationDistributions[1].VariantID)
+	assert.Equal(t, variant2.Key, evaluationDistributions[1].VariantKey)
+	assert.Equal(t, float32(20.00), evaluationDistributions[1].Rollout)
+
+	// update and re-evaluate 100 times
+	for i := 0; i < 100; i++ {
+		// update dist1 with same values
+		_, err = ruleStore.UpdateDistribution(context.TODO(), &flipt.UpdateDistributionRequest{
+			Id:        dist1.Id,
+			FlagKey:   flag.Key,
+			RuleId:    rule.Id,
+			VariantId: variant1.Id,
+			Rollout:   80.00,
+		})
+
+		require.NoError(t, err)
+
+		// update dist2 with same values
+		_, err = ruleStore.UpdateDistribution(context.TODO(), &flipt.UpdateDistributionRequest{
+			Id:        dist2.Id,
+			FlagKey:   flag.Key,
+			RuleId:    rule.Id,
+			VariantId: variant2.Id,
+			Rollout:   20.00,
+		})
+
+		require.NoError(t, err)
+
+		evaluationDistributions, err = evaluationStore.GetEvaluationDistributions(context.TODO(), rule.Id)
+		require.NoError(t, err)
+
+		assert.Equal(t, 2, len(evaluationDistributions))
+
+		assert.NotEmpty(t, evaluationDistributions[0].ID)
+		assert.Equal(t, rule.Id, evaluationDistributions[0].RuleID)
+		assert.Equal(t, variant1.Id, evaluationDistributions[0].VariantID)
+		assert.Equal(t, variant1.Key, evaluationDistributions[0].VariantKey)
+		assert.Equal(t, float32(80.00), evaluationDistributions[0].Rollout)
+
+		assert.NotEmpty(t, evaluationDistributions[1].ID)
+		assert.Equal(t, rule.Id, evaluationDistributions[1].RuleID)
+		assert.Equal(t, variant2.Id, evaluationDistributions[1].VariantID)
+		assert.Equal(t, variant2.Key, evaluationDistributions[1].VariantKey)
+		assert.Equal(t, float32(20.00), evaluationDistributions[1].Rollout)
+	}
+}