Skip to content

Commit 26db339

Browse files
authored
fix: use the updated schema (#183)
* fix: use the updated schema * use old evaluation * make getMatchingSegmentsAndOverrides easier on the eyes * use string instead of string pointer for reason field * don't use pointer for metadata
1 parent a8501c0 commit 26db339

File tree

9 files changed

+127
-186
lines changed

9 files changed

+127
-186
lines changed

flagengine/engine.go

Lines changed: 67 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -22,62 +22,67 @@ func getPriorityOrDefault(priority *float64) float64 {
2222
}
2323

2424
func getMatchingSegmentsAndOverrides(ec *engine_eval.EngineEvaluationContext) ([]engine_eval.SegmentResult, map[string]featureContextWithSegmentName) {
25-
segments := []engine_eval.SegmentResult{}
26-
segmentFeatureContexts := make(map[string]featureContextWithSegmentName)
25+
segmentResults := []engine_eval.SegmentResult{}
26+
featureOverrides := make(map[string]featureContextWithSegmentName)
2727

28-
// Get sorted segment keys for deterministic ordering
29-
segmentKeys := make([]string, 0, len(ec.Segments))
30-
for key := range ec.Segments {
31-
segmentKeys = append(segmentKeys, key)
32-
}
33-
sort.Strings(segmentKeys)
34-
35-
// Process segments in sorted order
36-
for _, key := range segmentKeys {
37-
segmentContext := ec.Segments[key]
28+
// Process segments in deterministic order (sorted by key)
29+
for _, segmentContext := range getSortedSegments(ec.Segments) {
3830
if !engine_eval.IsContextInSegment(ec, &segmentContext) {
3931
continue
4032
}
4133

42-
// Add segment to results
43-
segments = append(segments, engine_eval.SegmentResult{
44-
Key: segmentContext.Key,
34+
// Record matched segment
35+
segmentResults = append(segmentResults, engine_eval.SegmentResult{
4536
Name: segmentContext.Name,
4637
Metadata: segmentContext.Metadata,
4738
})
4839

49-
// Process segment overrides
50-
if segmentContext.Overrides != nil {
51-
for i := range segmentContext.Overrides {
52-
override := &segmentContext.Overrides[i]
53-
featureKey := override.FeatureKey
54-
55-
// Check if we should update the segment feature context
56-
shouldUpdate := false
57-
if existing, exists := segmentFeatureContexts[featureKey]; !exists {
58-
shouldUpdate = true
59-
} else {
60-
existingPriority := getPriorityOrDefault(existing.featureContext.Priority)
61-
overridePriority := getPriorityOrDefault(override.Priority)
62-
if overridePriority < existingPriority {
63-
shouldUpdate = true
64-
}
65-
}
40+
// Apply segment's feature overrides (respecting priority)
41+
applySegmentOverrides(&segmentContext, featureOverrides)
42+
}
6643

67-
if shouldUpdate {
68-
segmentFeatureContexts[featureKey] = featureContextWithSegmentName{
69-
featureContext: override,
70-
segmentName: segmentContext.Name,
71-
}
72-
}
44+
return segmentResults, featureOverrides
45+
}
46+
47+
// getSortedSegments returns segments sorted by their keys for deterministic ordering.
48+
func getSortedSegments(segments map[string]engine_eval.SegmentContext) []engine_eval.SegmentContext {
49+
keys := make([]string, 0, len(segments))
50+
for key := range segments {
51+
keys = append(keys, key)
52+
}
53+
sort.Strings(keys)
54+
55+
sorted := make([]engine_eval.SegmentContext, 0, len(keys))
56+
for _, key := range keys {
57+
sorted = append(sorted, segments[key])
58+
}
59+
return sorted
60+
}
61+
62+
// applySegmentOverrides updates the feature overrides map with this segment's overrides,
63+
// only replacing existing overrides if the new one has equal or higher priority.
64+
func applySegmentOverrides(segment *engine_eval.SegmentContext, featureOverrides map[string]featureContextWithSegmentName) {
65+
for i := range segment.Overrides {
66+
override := &segment.Overrides[i]
67+
newPriority := getPriorityOrDefault(override.Priority)
68+
69+
// Check if we should use this override
70+
if existing, exists := featureOverrides[override.Name]; exists {
71+
existingPriority := getPriorityOrDefault(existing.featureContext.Priority)
72+
if newPriority > existingPriority {
73+
continue // Existing override has higher priority
7374
}
7475
}
75-
}
7676

77-
return segments, segmentFeatureContexts
77+
// Use this override (either it's new or has equal/higher priority)
78+
featureOverrides[override.Name] = featureContextWithSegmentName{
79+
featureContext: override,
80+
segmentName: segment.Name,
81+
}
82+
}
7883
}
7984

80-
func getFlagResults(ec *engine_eval.EngineEvaluationContext, segmentFeatureContexts map[string]featureContextWithSegmentName) map[string]*engine_eval.FlagResult {
85+
func getFlagResults(ec *engine_eval.EngineEvaluationContext, featureOverrides map[string]featureContextWithSegmentName) map[string]*engine_eval.FlagResult {
8186
flags := make(map[string]*engine_eval.FlagResult)
8287

8388
// Get identity key if identity exists
@@ -87,24 +92,20 @@ func getFlagResults(ec *engine_eval.EngineEvaluationContext, segmentFeatureConte
8792
}
8893

8994
if ec.Features != nil {
90-
for _, featureContext := range ec.Features {
91-
// Check if we have a segment override for this feature
92-
if segmentFeatureCtx, exists := segmentFeatureContexts[featureContext.FeatureKey]; exists {
93-
// Use segment override
94-
fc := segmentFeatureCtx.featureContext
95-
reason := fmt.Sprintf("TARGETING_MATCH; segment=%s", segmentFeatureCtx.segmentName)
96-
flags[featureContext.Name] = &engine_eval.FlagResult{
97-
Enabled: fc.Enabled,
98-
FeatureKey: fc.FeatureKey,
99-
Name: fc.Name,
100-
Reason: &reason,
101-
Value: fc.Value,
102-
Metadata: fc.Metadata,
95+
for featureName, featureContext := range ec.Features {
96+
// Check if there's an override for this feature
97+
if override, ok := featureOverrides[featureName]; ok {
98+
flags[featureName] = &engine_eval.FlagResult{
99+
Enabled: override.featureContext.Enabled,
100+
Name: featureName,
101+
Reason: fmt.Sprintf("TARGETING_MATCH; segment=%s", override.segmentName),
102+
Value: override.featureContext.Value,
103+
Metadata: override.featureContext.Metadata,
103104
}
104105
} else {
105106
// Use default feature context
106-
flagResult := getFlagResultFromFeatureContext(&featureContext, identityKey)
107-
flags[featureContext.Name] = &flagResult
107+
flagResult := getFlagResultFromFeatureContext(featureName, &featureContext, identityKey)
108+
flags[featureName] = &flagResult
108109
}
109110
}
110111
}
@@ -114,20 +115,20 @@ func getFlagResults(ec *engine_eval.EngineEvaluationContext, segmentFeatureConte
114115

115116
// GetEvaluationResult computes flags and matched segments.
116117
func GetEvaluationResult(ec *engine_eval.EngineEvaluationContext) engine_eval.EvaluationResult {
117-
// Process segments
118-
segments, segmentFeatureContexts := getMatchingSegmentsAndOverrides(ec)
118+
// Process segments and get overrides
119+
segmentResults, featureOverrides := getMatchingSegmentsAndOverrides(ec)
119120

120121
// Get flag results
121-
flags := getFlagResults(ec, segmentFeatureContexts)
122+
flags := getFlagResults(ec, featureOverrides)
122123

123124
return engine_eval.EvaluationResult{
124125
Flags: flags,
125-
Segments: segments,
126+
Segments: segmentResults,
126127
}
127128
}
128129

129130
// getFlagResultFromFeatureContext creates a FlagResult from a FeatureContext.
130-
func getFlagResultFromFeatureContext(featureContext *engine_eval.FeatureContext, identityKey *string) engine_eval.FlagResult {
131+
func getFlagResultFromFeatureContext(featureName string, featureContext *engine_eval.FeatureContext, identityKey *string) engine_eval.FlagResult {
131132
reason := "DEFAULT"
132133
value := featureContext.Value
133134

@@ -153,12 +154,11 @@ func getFlagResultFromFeatureContext(featureContext *engine_eval.FeatureContext,
153154
}
154155

155156
flagResult := engine_eval.FlagResult{
156-
Enabled: featureContext.Enabled,
157-
FeatureKey: featureContext.FeatureKey,
158-
Name: featureContext.Name,
159-
Value: value,
160-
Reason: &reason,
161-
Metadata: featureContext.Metadata,
157+
Enabled: featureContext.Enabled,
158+
Name: featureName,
159+
Value: value,
160+
Reason: reason,
161+
Metadata: featureContext.Metadata,
162162
}
163163

164164
return flagResult

flagengine/engine_eval/context.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type EngineEvaluationContext struct {
2222
//
2323
// Represents an environment context for feature flag evaluation.
2424
type EnvironmentContext struct {
25-
// An environment's unique identifier.
25+
// Unique environment key. May be used for selecting a value for a multivariate feature, or for % split segmentation
2626
Key string `json:"key"`
2727
// An environment's human-readable name.
2828
Name string `json:"name"`
@@ -32,8 +32,6 @@ type EnvironmentContext struct {
3232
type FeatureContext struct {
3333
// Indicates whether the feature is enabled in the environment.
3434
Enabled bool `json:"enabled"`
35-
// Unique feature identifier.
36-
FeatureKey string `json:"feature_key"`
3735
// Key used when selecting a value for a multivariate feature. Set to an internal identifier
3836
// or a UUID, depending on Flagsmith implementation.
3937
Key string `json:"key"`
@@ -49,7 +47,7 @@ type FeatureContext struct {
4947
// value for standard features, or multiple values for multivariate features.
5048
Variants []FeatureValue `json:"variants,omitempty"`
5149
// Metadata about the feature.
52-
Metadata *FeatureMetadata `json:"metadata,omitempty"`
50+
Metadata FeatureMetadata `json:"metadata,omitempty"`
5351
}
5452

5553
// Represents a multivariate value for a feature flag.
@@ -133,7 +131,7 @@ type SegmentContext struct {
133131
// The name of the segment.
134132
Name string `json:"name"`
135133
// Metadata about the segment.
136-
Metadata *SegmentMetadata `json:"metadata,omitempty"`
134+
Metadata SegmentMetadata `json:"metadata,omitempty"`
137135
// Feature overrides for the segment.
138136
Overrides []FeatureContext `json:"overrides,omitempty"`
139137
// Rules that define the segment.

flagengine/engine_eval/evaluator_test.go

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func createSegmentContext(key, name string, rules []engine_eval.SegmentRule) *en
6161
return &engine_eval.SegmentContext{
6262
Key: key,
6363
Name: name,
64-
Metadata: &engine_eval.SegmentMetadata{
64+
Metadata: engine_eval.SegmentMetadata{
6565
SegmentID: segmentID,
6666
Source: engine_eval.SegmentSourceAPI,
6767
},
@@ -514,26 +514,6 @@ func TestGetContextValueIntegration(t *testing.T) {
514514
result := engine_eval.IsContextInSegment(evalContext, segmentContext)
515515
assert.True(t, result)
516516
})
517-
518-
t.Run("JSONPath environment key works", func(t *testing.T) {
519-
evalContext := createEvaluationContext(nil)
520-
521-
segmentContext := createSegmentContext("test", "test", []engine_eval.SegmentRule{
522-
{
523-
Type: engine_eval.All,
524-
Conditions: []engine_eval.Condition{
525-
{
526-
Operator: engine_eval.Equal,
527-
Property: "$.environment.key",
528-
Value: "test-env",
529-
},
530-
},
531-
},
532-
})
533-
534-
result := engine_eval.IsContextInSegment(evalContext, segmentContext)
535-
assert.True(t, result)
536-
})
537517
}
538518

539519
func TestToStringIntegration(t *testing.T) {

flagengine/engine_eval/mappers.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,10 @@ func mapFeatureStateToFeatureContext(fs *features.FeatureStateModel) FeatureCont
8686
}
8787

8888
fc := FeatureContext{
89-
Enabled: fs.Enabled,
90-
FeatureKey: strconv.Itoa(fs.Feature.ID),
91-
Key: key,
92-
Name: fs.Feature.Name,
93-
Metadata: &FeatureMetadata{
89+
Enabled: fs.Enabled,
90+
Key: key,
91+
Name: fs.Feature.Name,
92+
Metadata: FeatureMetadata{
9493
FeatureID: fs.Feature.ID,
9594
},
9695
}
@@ -116,7 +115,7 @@ func mapSegmentToSegmentContext(s *segments.SegmentModel) SegmentContext {
116115
sc := SegmentContext{
117116
Key: strconv.Itoa(s.ID),
118117
Name: s.Name,
119-
Metadata: &SegmentMetadata{
118+
Metadata: SegmentMetadata{
120119
SegmentID: s.ID,
121120
Source: SegmentSourceAPI,
122121
},
@@ -242,7 +241,7 @@ func mapIdentityOverridesToSegments(identityOverrides []*identities.IdentityMode
242241
sc := SegmentContext{
243242
Key: "", // Identity override segments never use % Split operator
244243
Name: "identity_overrides",
245-
Metadata: &SegmentMetadata{
244+
Metadata: SegmentMetadata{
246245
Source: SegmentSourceIdentityOverride,
247246
},
248247
Rules: []SegmentRule{
@@ -264,13 +263,12 @@ func mapIdentityOverridesToSegments(identityOverrides []*identities.IdentityMode
264263
priority := math.Inf(-1) // Strongest possible priority
265264
featureID := featureNameToID[override.featureName]
266265
featureOverride := FeatureContext{
267-
Key: "", // Identity overrides never carry multivariate options
268-
FeatureKey: strconv.Itoa(featureID),
269-
Name: override.featureName,
270-
Enabled: override.enabled,
271-
Priority: &priority,
272-
Value: override.featureValue,
273-
Metadata: &FeatureMetadata{
266+
Key: "", // Identity overrides never carry multivariate options
267+
Name: override.featureName,
268+
Enabled: override.enabled,
269+
Priority: &priority,
270+
Value: override.featureValue,
271+
Metadata: FeatureMetadata{
274272
FeatureID: featureID,
275273
},
276274
}
@@ -336,7 +334,7 @@ func MapEvaluationResultSegmentsToSegmentModels(
336334

337335
for _, segmentResult := range result.Segments {
338336
// Only include segments from API source (filter out identity overrides)
339-
if segmentResult.Metadata == nil || segmentResult.Metadata.Source != SegmentSourceAPI {
337+
if segmentResult.Metadata.Source != SegmentSourceAPI {
340338
continue
341339
}
342340

flagengine/engine_eval/mappers_test.go

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,6 @@ func TestMapEnvironmentDocumentToEvaluationContext(t *testing.T) {
9898
if !testFeature.Enabled {
9999
t.Error("Expected test-feature to be enabled")
100100
}
101-
if testFeature.FeatureKey != "1" {
102-
t.Errorf("Expected FeatureKey to be '1' (feature ID), got %v", testFeature.FeatureKey)
103-
}
104101
if testFeature.Name != "test-feature" {
105102
t.Errorf("Expected Name to be 'test-feature', got %v", testFeature.Name)
106103
}
@@ -174,9 +171,6 @@ func TestMapEnvironmentDocumentToEvaluationContext(t *testing.T) {
174171
t.Errorf("Expected 1 override in segment, got %d", len(testSegment.Overrides))
175172
} else {
176173
override := testSegment.Overrides[0]
177-
if override.FeatureKey != "1" {
178-
t.Errorf("Expected override feature key to be '1' (feature ID), got %v", override.FeatureKey)
179-
}
180174
if !override.Enabled {
181175
t.Error("Expected segment override to be enabled")
182176
}
@@ -386,9 +380,8 @@ func TestMapContextAndIdentityDataToContext(t *testing.T) {
386380
},
387381
Features: map[string]FeatureContext{
388382
"test-feature": {
389-
Enabled: true,
390-
FeatureKey: "1",
391-
Name: "test-feature",
383+
Enabled: true,
384+
Name: "test-feature",
392385
},
393386
},
394387
}
@@ -527,25 +520,22 @@ func TestMapEvaluationResultSegmentsToSegmentModels(t *testing.T) {
527520
result := EvaluationResult{
528521
Segments: []SegmentResult{
529522
{
530-
Key: "1",
531523
Name: "test-segment",
532-
Metadata: &SegmentMetadata{
524+
Metadata: SegmentMetadata{
533525
SegmentID: 1,
534526
Source: SegmentSourceAPI,
535527
},
536528
},
537529
{
538-
Key: "42",
539530
Name: "another-segment",
540-
Metadata: &SegmentMetadata{
531+
Metadata: SegmentMetadata{
541532
SegmentID: 42,
542533
Source: SegmentSourceAPI,
543534
},
544535
},
545536
{
546-
Key: "",
547537
Name: "identity-override-segment",
548-
Metadata: &SegmentMetadata{
538+
Metadata: SegmentMetadata{
549539
SegmentID: 0,
550540
Source: SegmentSourceIdentityOverride,
551541
},
@@ -609,7 +599,6 @@ func TestMapEvaluationResultSegmentsToSegmentModelsInvalidKey(t *testing.T) {
609599
result := EvaluationResult{
610600
Segments: []SegmentResult{
611601
{
612-
Key: "invalid-key",
613602
Name: "segment-without-metadata",
614603
},
615604
},

0 commit comments

Comments
 (0)