Skip to content

Commit 6a5b95f

Browse files
ploomizploomiz
authored
Fix deserialization of targeting rules (#14)
1 parent e687a38 commit 6a5b95f

File tree

8 files changed

+92
-118
lines changed

8 files changed

+92
-118
lines changed

‎.github/workflows/test-sdk.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ jobs:
1111
- uses: actions/setup-go@v3
1212
with:
1313
go-version: 1.18
14-
1514
- name: Build
1615
run: go build -v ./...
17-
16+
- name: 'Set up GCP SDK for downloading test data'
17+
uses: 'google-github-actions/setup-gcloud@v0'
1818
- name: Test
19-
run: go test -v ./...
19+
run: make test

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,6 @@ eppo-golang-sdk-*
1414

1515
# Dependency directories (remove the comment below to include it)
1616
# vendor/
17-
eppoclient/test-data/assignment/*
17+
eppoclient/test-data/assignment-v2
18+
eppoclient/test-data/rac-experiments.json
1819
.vscode

‎Makefile‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@ help: Makefile
1212
@echo "usage: make <target>"
1313
@sed -n 's/^##//p' $<
1414

15-
test:
15+
testDataDir := eppoclient/test-data/
16+
.PHONY: test-data
17+
test-data:
18+
rm -rf $(testDataDir)
19+
mkdir -p $(testDataDir)
20+
gsutil cp gs://sdk-test-data/rac-experiments.json $(testDataDir)
21+
gsutil cp -r gs://sdk-test-data/assignment-v2 $(testDataDir)
22+
23+
test: test-data
1624
go test ./...
1725

1826
lint:

‎eppoclient/client_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ func Test_AssignSubjectWithAttributesAndRules(t *testing.T) {
112112
var mockLogger = new(mockLogger)
113113
mockLogger.Mock.On("LogAssignment", mock.Anything).Return()
114114

115-
var matchesEmailCondition = condition{operator: "MATCHES", value: ".*@eppo.com", attribute: "email"}
116-
var textRule = rule{conditions: []condition{matchesEmailCondition}}
115+
var matchesEmailCondition = condition{Operator: "MATCHES", Value: ".*@eppo.com", Attribute: "email"}
116+
var textRule = rule{Conditions: []condition{matchesEmailCondition}}
117117
var mockConfigRequestor = new(mockConfigRequestor)
118118
var overrides = make(dictionary)
119119
var mockVariations = []Variation{

‎eppoclient/eppoclient_e2e_test.go‎

Lines changed: 18 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,21 @@
11
package eppoclient
22

33
import (
4-
"context"
54
"encoding/json"
65
"fmt"
7-
"io"
86
"io/ioutil"
9-
"log"
107
"net/http"
118
"net/http/httptest"
129
"os"
1310
"strings"
1411
"testing"
1512
"time"
1613

17-
"cloud.google.com/go/storage"
1814
"github.com/stretchr/testify/assert"
19-
"google.golang.org/api/iterator"
20-
"google.golang.org/api/option"
2115
)
2216

23-
const TEST_DATA_DIR = "test-data/assignment"
24-
const BUCKET_NAME = "sdk-test-data"
17+
const TEST_DATA_DIR = "test-data/assignment-v2"
18+
const MOCK_RAC_RESPONSE_FILE = "test-data/rac-experiments.json"
2519

2620
var tstData = []testData{}
2721

@@ -38,6 +32,16 @@ func Test_e2e(t *testing.T) {
3832

3933
assignments := []string{}
4034

35+
for _, subject := range experiment.SubjectsWithAttributes {
36+
assignment, err := client.GetAssignment(subject.SubjectKey, expName, subject.SubjectAttributes)
37+
38+
if assignment != "" {
39+
assert.Nil(t, err)
40+
}
41+
42+
assignments = append(assignments, assignment)
43+
}
44+
4145
for _, subject := range experiment.Subjects {
4246
assignment, err := client.GetAssignment(subject, expName, dictionary{})
4347

@@ -53,7 +57,6 @@ func Test_e2e(t *testing.T) {
5357
}
5458

5559
func initFixture() string {
56-
downloadTestData()
5760
testResponse := getTestData() // this is here because we need to append to global testData
5861

5962
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -90,67 +93,12 @@ func getTestData() dictionary {
9093
tstData = append(tstData, testCaseDict)
9194
}
9295

93-
expConfigs := dictionary{}
94-
95-
for _, experimentTest := range tstData {
96-
experimentName := experimentTest.Experiment
97-
expMap := dictionary{}
98-
expMap["subjectShards"] = 10000
99-
expMap["enabled"] = true
100-
expMap["variations"] = experimentTest.Variations
101-
expMap["name"] = experimentName
102-
expMap["percentExposure"] = experimentTest.PercentExposure
103-
104-
expConfigs[experimentName] = expMap
105-
}
106-
107-
response := dictionary{}
108-
response["experiments"] = expConfigs
109-
110-
return response
111-
}
112-
113-
func downloadTestData() {
114-
if _, err := os.Stat(TEST_DATA_DIR); os.IsNotExist(err) {
115-
if err := os.MkdirAll(TEST_DATA_DIR, os.ModePerm); err != nil {
116-
log.Fatal(err)
117-
}
118-
} else {
119-
return //data is already downloaded, skip this step
120-
}
121-
122-
ctx := context.Background()
123-
client, err := storage.NewClient(ctx, option.WithoutAuthentication())
96+
var racResponseData map[string]interface{}
97+
racResponseJsonFile, _ := os.Open(MOCK_RAC_RESPONSE_FILE)
98+
byteValue, _ := ioutil.ReadAll(racResponseJsonFile)
99+
err = json.Unmarshal(byteValue, &racResponseData)
124100
if err != nil {
125-
fmt.Println(err)
126-
}
127-
128-
query := &storage.Query{Prefix: "assignment/test-case"}
129-
bkt := client.Bucket(BUCKET_NAME)
130-
it := bkt.Objects(ctx, query)
131-
132-
for {
133-
attrs, err := it.Next()
134-
if err == iterator.Done {
135-
break
136-
}
137-
if err != nil {
138-
log.Fatal(err)
139-
}
140-
obj := bkt.Object(attrs.Name)
141-
rdr, err := obj.NewReader(ctx)
142-
143-
if err != nil {
144-
log.Fatal(err)
145-
}
146-
defer rdr.Close()
147-
148-
out, err := os.Create("test-data/" + obj.ObjectName())
149-
if err != nil {
150-
log.Fatal(err)
151-
}
152-
defer out.Close()
153-
154-
io.Copy(out, rdr)
101+
fmt.Println("Error reading mock RAC response file")
155102
}
103+
return racResponseData
156104
}

‎eppoclient/rules.go‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ import (
99
)
1010

1111
type condition struct {
12-
attribute string
13-
value interface{}
14-
operator string `validator:"regexp=^(MATCHES|GTE|GT|LTE|LT|ONE_OF|NOT_ONE_OF)$"`
12+
Attribute string `json:"attribute"`
13+
Value interface{} `json:"value"`
14+
Operator string `validator:"regexp=^(MATCHES|GTE|GT|LTE|LT|ONE_OF|NOT_ONE_OF)$" json:"operator"`
1515
}
1616

1717
type rule struct {
18-
conditions []condition
18+
Conditions []condition `json:"conditions"`
1919
}
2020

2121
func matchesAnyRule(subjectAttributes dictionary, rules []rule) bool {
@@ -29,7 +29,7 @@ func matchesAnyRule(subjectAttributes dictionary, rules []rule) bool {
2929
}
3030

3131
func matchesRule(subjectAttributes dictionary, rule rule) bool {
32-
for _, condition := range rule.conditions {
32+
for _, condition := range rule.Conditions {
3333
if !evaluateCondition(subjectAttributes, condition) {
3434
return false
3535
}
@@ -39,27 +39,38 @@ func matchesRule(subjectAttributes dictionary, rule rule) bool {
3939
}
4040

4141
func evaluateCondition(subjectAttributes dictionary, condition condition) bool {
42-
subjectValue := subjectAttributes[condition.attribute]
42+
subjectValue := subjectAttributes[condition.Attribute]
4343

4444
if subjectValue != nil {
45-
if condition.operator == "MATCHES" {
45+
if condition.Operator == "MATCHES" {
4646
v := reflect.ValueOf(subjectValue)
4747
if v.Kind() != reflect.String {
4848
subjectValue = strconv.Itoa(subjectValue.(int))
4949
}
50-
r, _ := regexp.MatchString(condition.value.(string), subjectValue.(string))
50+
r, _ := regexp.MatchString(condition.Value.(string), subjectValue.(string))
5151
return r
52-
} else if condition.operator == "ONE_OF" {
53-
return isOneOf(subjectValue, condition.value.([]string))
54-
} else if condition.operator == "NOT_ONE_OF" {
55-
return isNotOneOf(subjectValue, condition.value.([]string))
52+
} else if condition.Operator == "ONE_OF" {
53+
return isOneOf(subjectValue, convertToStringArray(condition.Value))
54+
} else if condition.Operator == "NOT_ONE_OF" {
55+
return isNotOneOf(subjectValue, convertToStringArray(condition.Value))
5656
} else {
5757
return evaluateNumericCondition(subjectValue, condition)
5858
}
5959
}
6060
return false
6161
}
6262

63+
func convertToStringArray(conditionValue interface{}) []string {
64+
if reflect.TypeOf(conditionValue).Elem().Kind() == reflect.String {
65+
return conditionValue.([]string)
66+
}
67+
conditionValueStrings := make([]string, len(conditionValue.([]interface{})))
68+
for i, v := range conditionValue.([]interface{}) {
69+
conditionValueStrings[i] = v.(string)
70+
}
71+
return conditionValueStrings
72+
}
73+
6374
func isOneOf(attributeValue interface{}, conditionValue []string) bool {
6475
matches := getMatchingStringValues(attributeValue, conditionValue)
6576
return len(matches) > 0
@@ -99,15 +110,15 @@ func evaluateNumericCondition(subjectValue interface{}, condition condition) boo
99110
subjectValue = float64(subjectValue.(int))
100111
}
101112

102-
switch condition.operator {
113+
switch condition.Operator {
103114
case "GT":
104-
return subjectValue.(float64) > condition.value.(float64)
115+
return subjectValue.(float64) > condition.Value.(float64)
105116
case "GTE":
106-
return subjectValue.(float64) >= condition.value.(float64)
117+
return subjectValue.(float64) >= condition.Value.(float64)
107118
case "LT":
108-
return subjectValue.(float64) < condition.value.(float64)
119+
return subjectValue.(float64) < condition.Value.(float64)
109120
case "LTE":
110-
return subjectValue.(float64) <= condition.value.(float64)
121+
return subjectValue.(float64) <= condition.Value.(float64)
111122
default:
112123
panic("Incorrect condition operator")
113124
}

‎eppoclient/rules_test.go‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ import (
66
"github.com/stretchr/testify/assert"
77
)
88

9-
var greaterThanCondition = condition{operator: "GT", value: 10.0, attribute: "age"}
10-
var lessThanCondition = condition{operator: "LT", value: 100.0, attribute: "age"}
11-
var numericRule = rule{conditions: []condition{greaterThanCondition, lessThanCondition}}
9+
var greaterThanCondition = condition{Operator: "GT", Value: 10.0, Attribute: "age"}
10+
var lessThanCondition = condition{Operator: "LT", Value: 100.0, Attribute: "age"}
11+
var numericRule = rule{Conditions: []condition{greaterThanCondition, lessThanCondition}}
1212

13-
var matchesEmailCondition = condition{operator: "MATCHES", value: ".*@email.com", attribute: "email"}
14-
var textRule = rule{conditions: []condition{matchesEmailCondition}}
15-
var ruleWithEmptyConditions = rule{conditions: []condition{}}
13+
var matchesEmailCondition = condition{Operator: "MATCHES", Value: ".*@email.com", Attribute: "email"}
14+
var textRule = rule{Conditions: []condition{matchesEmailCondition}}
15+
var ruleWithEmptyConditions = rule{Conditions: []condition{}}
1616

1717
func Test_matchesAnyRule_withEmptyRules(t *testing.T) {
1818
expected := false
@@ -84,8 +84,8 @@ func Test_matchesAnyRule_NumericOperatorWithString(t *testing.T) {
8484
func Test_matchesAnyRule_NumericValueAndRegex(t *testing.T) {
8585
expected := true
8686

87-
cdn := condition{operator: "MATCHES", value: "[0-9]+", attribute: "age"}
88-
rl := rule{conditions: []condition{cdn}}
87+
cdn := condition{Operator: "MATCHES", Value: "[0-9]+", Attribute: "age"}
88+
rl := rule{Conditions: []condition{cdn}}
8989

9090
subjectAttributes := make(dictionary)
9191
subjectAttributes["age"] = 99
@@ -102,8 +102,8 @@ type MatchesAnyRuleTest []struct {
102102
}
103103

104104
func Test_matchesAnyRule_oneOfOperatorWithBoolean(t *testing.T) {
105-
oneOfRule := rule{conditions: []condition{{operator: "ONE_OF", value: []string{"true"}, attribute: "enabled"}}}
106-
notOneOfRule := rule{conditions: []condition{{operator: "NOT_ONE_OF", value: []string{"True"}, attribute: "enabled"}}}
105+
oneOfRule := rule{Conditions: []condition{{Operator: "ONE_OF", Value: []string{"true"}, Attribute: "enabled"}}}
106+
notOneOfRule := rule{Conditions: []condition{{Operator: "NOT_ONE_OF", Value: []string{"True"}, Attribute: "enabled"}}}
107107

108108
subjectAttributesEnabled := make(dictionary)
109109
subjectAttributesEnabled["enabled"] = "true"
@@ -126,7 +126,7 @@ func Test_matchesAnyRule_oneOfOperatorWithBoolean(t *testing.T) {
126126
}
127127

128128
func Test_matchesAnyRule_OneOfOperatorCaseInsensitive(t *testing.T) {
129-
oneOfRule := rule{conditions: []condition{{operator: "ONE_OF", value: []string{"1Ab", "Ron"}, attribute: "name"}}}
129+
oneOfRule := rule{Conditions: []condition{{Operator: "ONE_OF", Value: []string{"1Ab", "Ron"}, Attribute: "name"}}}
130130
subjectAttributes0 := make(dictionary)
131131
subjectAttributes0["name"] = "ron"
132132

@@ -146,7 +146,7 @@ func Test_matchesAnyRule_OneOfOperatorCaseInsensitive(t *testing.T) {
146146
}
147147

148148
func Test_matchesAnyRule_NotOneOfOperatorCaseInsensitive(t *testing.T) {
149-
notOneOfRule := rule{conditions: []condition{{operator: "NOT_ONE_OF", value: []string{"bbB", "1.1.ab"}, attribute: "name"}}}
149+
notOneOfRule := rule{Conditions: []condition{{Operator: "NOT_ONE_OF", Value: []string{"bbB", "1.1.ab"}, Attribute: "name"}}}
150150
subjectAttributes0 := make(dictionary)
151151
subjectAttributes0["name"] = "BBB"
152152

@@ -166,8 +166,8 @@ func Test_matchesAnyRule_NotOneOfOperatorCaseInsensitive(t *testing.T) {
166166
}
167167

168168
func Test_matchesAnyRule_OneOfOperatorWithString(t *testing.T) {
169-
oneOfRule := rule{conditions: []condition{{operator: "ONE_OF", value: []string{"john", "ron"}, attribute: "name"}}}
170-
notOneOfRule := rule{conditions: []condition{{operator: "NOT_ONE_OF", value: []string{"ron"}, attribute: "name"}}}
169+
oneOfRule := rule{Conditions: []condition{{Operator: "ONE_OF", Value: []string{"john", "ron"}, Attribute: "name"}}}
170+
notOneOfRule := rule{Conditions: []condition{{Operator: "NOT_ONE_OF", Value: []string{"ron"}, Attribute: "name"}}}
171171

172172
subjectAttributesJohn := make(dictionary)
173173
subjectAttributesJohn["name"] = "john"
@@ -194,8 +194,8 @@ func Test_matchesAnyRule_OneOfOperatorWithString(t *testing.T) {
194194
}
195195

196196
func Test_matchesAnyRule_OneOfOperatorWithNumber(t *testing.T) {
197-
oneOfRule := rule{conditions: []condition{{operator: "ONE_OF", value: []string{"14", "15.11", "15"}, attribute: "number"}}}
198-
notOneOfRule := rule{conditions: []condition{{operator: "NOT_ONE_OF", value: []string{"10"}, attribute: "number"}}}
197+
oneOfRule := rule{Conditions: []condition{{Operator: "ONE_OF", Value: []string{"14", "15.11", "15"}, Attribute: "number"}}}
198+
notOneOfRule := rule{Conditions: []condition{{Operator: "NOT_ONE_OF", Value: []string{"10"}, Attribute: "number"}}}
199199

200200
subjectAttributes0 := make(dictionary)
201201
subjectAttributes0["number"] = "14"
@@ -272,18 +272,18 @@ func Test_isNotOneOf_Fail(t *testing.T) {
272272

273273
func Test_evaluateNumericCondition_Success(t *testing.T) {
274274
expected := false
275-
result := evaluateNumericCondition(40, condition{operator: "LT", value: 30.0})
275+
result := evaluateNumericCondition(40, condition{Operator: "LT", Value: 30.0})
276276

277277
assert.Equal(t, expected, result)
278278
}
279279

280280
func Test_evaluateNumericCondition_Fail(t *testing.T) {
281281
expected := true
282-
result := evaluateNumericCondition(25, condition{operator: "LT", value: 30.0})
282+
result := evaluateNumericCondition(25, condition{Operator: "LT", Value: 30.0})
283283

284284
assert.Equal(t, expected, result)
285285
}
286286

287287
func Test_evaluateNumericCondition_IncorrectOperator(t *testing.T) {
288-
assert.Panics(t, func() { evaluateNumericCondition(25, condition{operator: "LTGT", value: 30.0}) })
288+
assert.Panics(t, func() { evaluateNumericCondition(25, condition{Operator: "LTGT", Value: 30.0}) })
289289
}

‎eppoclient/utils.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,17 @@ package eppoclient
33
type dictionary map[string]interface{}
44

55
type testData struct {
6-
Experiment string `json:"experiment"`
7-
PercentExposure float32 `json:"percentExposure"`
8-
Variations []testDataVariations `json:"variations"`
9-
Subjects []string `json:"subjects"`
10-
ExpectedAssignments []string `json:"expectedAssignments"`
6+
Experiment string `json:"experiment"`
7+
PercentExposure float32 `json:"percentExposure"`
8+
Variations []testDataVariations `json:"variations"`
9+
Subjects []string `json:"subjects"`
10+
SubjectsWithAttributes []subjectWithAttributes `json:"subjectsWithAttributes"`
11+
ExpectedAssignments []string `json:"expectedAssignments"`
12+
}
13+
14+
type subjectWithAttributes struct {
15+
SubjectKey string `json:"subjectKey"`
16+
SubjectAttributes dictionary `json:"subjectAttributes"`
1117
}
1218

1319
type testDataVariations struct {

0 commit comments

Comments
 (0)