Use group_ids rather than groups field for elastic search queries

nyaruka · Apr 4, 2022 · dde8230 · dde8230
1 parent 6644bbc
commit dde8230
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 20 deletions.
diff --git a/contactql/es/query.go b/contactql/es/query.go
@@ -15,6 +15,7 @@ import (
 // AssetMapper is used to map engine assets to however ES identifies them
 type AssetMapper interface {
 	Flow(assets.Flow) int64
+	Group(assets.Group) int64
 }
 
 // ToElasticQuery converts a contactql query to an Elastic query
@@ -308,9 +309,9 @@ func attributeConditionToElastic(env envs.Environment, resolver contactql.Resolv
 
 		switch c.Operator() {
 		case contactql.OpEqual:
-			return elastic.NewTermQuery("groups", group.UUID())
+			return elastic.NewTermQuery("group_ids", mapper.Group(group))
 		case contactql.OpNotEqual:
-			return not(elastic.NewTermQuery("groups", group.UUID()))
+			return not(elastic.NewTermQuery("group_ids", mapper.Group(group)))
 		default:
 			panic(fmt.Sprintf("unsupported group attribute operator: %s", c.Operator()))
 		}

diff --git a/contactql/es/query_test.go b/contactql/es/query_test.go
@@ -20,15 +20,20 @@ import (
 )
 
 type MockMapper struct {
-	flows map[assets.FlowUUID]int64
+	flows  map[assets.FlowUUID]int64
+	groups map[assets.GroupUUID]int64
 }
 
 func (m *MockMapper) Flow(f assets.Flow) int64 {
 	return m.flows[f.UUID()]
 }
 
-func newMockMapper(flows map[assets.FlowUUID]int64) *MockMapper {
-	return &MockMapper{flows}
+func (m *MockMapper) Group(g assets.Group) int64 {
+	return m.groups[g.UUID()]
+}
+
+func newMockMapper(flows map[assets.FlowUUID]int64, groups map[assets.GroupUUID]int64) *MockMapper {
+	return &MockMapper{flows, groups}
 }
 
 func newMockResolver() contactql.Resolver {
@@ -41,18 +46,27 @@ func newMockResolver() contactql.Resolver {
 			static.NewField("54c72635-d747-4e45-883c-099d57dd998e", "district", "District", assets.FieldTypeDistrict),
 			static.NewField("fde8f740-c337-421b-8abb-83b954897c80", "ward", "Ward", assets.FieldTypeWard),
 		},
+		[]assets.Flow{
+			static.NewFlow("c261165a-f5b0-40ba-b916-76fb49667a4f", "Registration", []byte(`{}`)),
+		},
 		[]assets.Group{
 			static.NewGroup("8de30b78-d9ef-4db2-b2e8-4f7b6aef64cf", "U-Reporters", ""),
 			static.NewGroup("cf51cf8d-94da-447a-b27e-a42a900c37a6", "Testers", ""),
 		},
-		[]assets.Flow{
-			static.NewFlow("c261165a-f5b0-40ba-b916-76fb49667a4f", "Registration", []byte(`{}`)),
-		},
 	)
 }
 
 func TestElasticQuery(t *testing.T) {
 	resolver := newMockResolver()
+	mapper := newMockMapper(
+		map[assets.FlowUUID]int64{
+			"c261165a-f5b0-40ba-b916-76fb49667a4f": 234, // Registration
+		},
+		map[assets.GroupUUID]int64{
+			"8de30b78-d9ef-4db2-b2e8-4f7b6aef64cf": 345, // U-Reporters
+			"cf51cf8d-94da-447a-b27e-a42a900c37a6": 456, // Testers
+		},
+	)
 
 	type testCase struct {
 		Description string          `json:"description"`
@@ -79,7 +93,7 @@ func TestElasticQuery(t *testing.T) {
 		parsed, err := contactql.ParseQuery(env, tc.Query, resolver)
 		require.NoError(t, err)
 
-		query := es.ToElasticQuery(env, newMockMapper(map[assets.FlowUUID]int64{"c261165a-f5b0-40ba-b916-76fb49667a4f": 234}), parsed)
+		query := es.ToElasticQuery(env, mapper, parsed)
 		assert.NotNil(t, query, tc.Description)
 
 		source, err := query.Source()

diff --git a/contactql/es/testdata/to_query.json b/contactql/es/testdata/to_query.json
@@ -1674,7 +1674,7 @@
         "query": "group = \"U-Reporters\"",
         "elastic": {
             "term": {
-                "groups": "8de30b78-d9ef-4db2-b2e8-4f7b6aef64cf"
+                "group_ids": 345
             }
         }
     },
@@ -1685,7 +1685,7 @@
             "bool": {
                 "must_not": {
                     "term": {
-                        "groups": "8de30b78-d9ef-4db2-b2e8-4f7b6aef64cf"
+                        "group_ids": 345
                     }
                 }
             }

diff --git a/contactql/evaluator_test.go b/contactql/evaluator_test.go
@@ -153,11 +153,11 @@ func TestEvaluateQuery(t *testing.T) {
 			static.NewField("023f733d-ce00-4a61-96e4-b411987028ea", "empty", "Empty", assets.FieldTypeText),
 			static.NewField("81e25783-a1d8-42b9-85e4-68c7ab2df39d", "xyz", "XYZ", assets.FieldTypeText),
 		},
-		[]assets.Group{},
 		[]assets.Flow{
 			static.NewFlow("ea351bf8-3c49-46dd-935c-5b20e2a00b7a", "Registration", []byte(`{}`)),
 			static.NewFlow("1b73528f-6e4e-4c64-b393-78088449fb49", "Catch All", []byte(`{}`)),
 		},
+		[]assets.Group{},
 	)
 
 	for _, test := range tests {

diff --git a/contactql/inspect_test.go b/contactql/inspect_test.go
@@ -19,10 +19,10 @@ func TestInspect(t *testing.T) {
 			static.NewField(assets.FieldUUID("3810a485-3fda-4011-a589-7320c0b8dbef"), "dob", "DOB", assets.FieldTypeDatetime),
 			static.NewField(assets.FieldUUID("d66a7823-eada-40e5-9a3a-57239d4690bf"), "gender", "Gender", assets.FieldTypeText),
 		},
+		[]assets.Flow{},
 		[]assets.Group{
 			static.NewGroup(assets.GroupUUID("4eeca453-f474-4767-bdd0-434b180223db"), "U-Reporters", ""),
 		},
-		[]assets.Flow{},
 	)
 
 	tests := []struct {

diff --git a/contactql/mock.go b/contactql/mock.go
@@ -8,16 +8,16 @@ import (
 
 type mockResolver struct {
 	fields []assets.Field
-	groups []assets.Group
 	flows  []assets.Flow
+	groups []assets.Group
 }
 
 // NewMockResolver creates a new mock resolver for fields and groups
-func NewMockResolver(fields []assets.Field, groups []assets.Group, flows []assets.Flow) Resolver {
+func NewMockResolver(fields []assets.Field, flows []assets.Flow, groups []assets.Group) Resolver {
 	return &mockResolver{
 		fields: fields,
-		groups: groups,
 		flows:  flows,
+		groups: groups,
 	}
 }
 

diff --git a/contactql/parser_test.go b/contactql/parser_test.go
@@ -18,12 +18,12 @@ func TestParseQuery(t *testing.T) {
 			static.NewField("165def68-3216-4ebf-96bc-f6f1ee5bd966", "state", "State", assets.FieldTypeState),
 			static.NewField("85baf5e1-b57a-46dc-a726-a84e8c4229c7", "dob", "DOB", assets.FieldTypeDatetime),
 		},
-		[]assets.Group{
-			static.NewGroup("a9b5b0a0-1098-4bc2-8384-eea09ae43e6b", "U-Reporters", ""),
-		},
 		[]assets.Flow{
 			static.NewFlow("f87fd7cd-e501-4394-9cff-62309af85138", "Registration", []byte(`{}`)),
 		},
+		[]assets.Group{
+			static.NewGroup("a9b5b0a0-1098-4bc2-8384-eea09ae43e6b", "U-Reporters", ""),
+		},
 	)
 
 	tests := []struct {
@@ -358,8 +358,8 @@ func TestParsingErrors(t *testing.T) {
 			static.NewField("3810a485-3fda-4011-a589-7320c0b8dbef", "dob", "DOB", assets.FieldTypeDatetime),
 			static.NewField("d66a7823-eada-40e5-9a3a-57239d4690bf", "gender", "Gender", assets.FieldTypeText),
 		},
-		[]assets.Group{},
 		[]assets.Flow{},
+		[]assets.Group{},
 	)
 
 	for _, tc := range tests {