From a4075e7004aad3c430ecf403a7bcc13c56a358b9 Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Fri, 13 Dec 2019 13:30:32 -0800 Subject: [PATCH 01/24] first stab at contact search API --- .gitignore | 1 + go.mod | 2 ++ go.sum | 7 +++++ mailroom.go | 2 +- models/contacts.go | 55 +++++++++++++++++++++++++++++++++++ web/search/search.go | 69 ++++++++++++++++++++++++++++++++++++++++++++ web/server.go | 31 +++++++++++--------- 7 files changed, 152 insertions(+), 15 deletions(-) create mode 100644 web/search/search.go diff --git a/.gitignore b/.gitignore index 3b14996bb..e1d7979d6 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ dist .vscode .envrc docs/* +docs # Test binary, build with `go test -c` *.test diff --git a/go.mod b/go.mod index 75712aebd..dae611d28 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ require ( github.com/google/go-cmp v0.3.0 // indirect github.com/gorilla/schema v1.0.2 github.com/jmoiron/sqlx v1.2.0 + github.com/kr/pretty v0.1.0 // indirect github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 // indirect github.com/lib/pq v1.0.0 github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e // indirect @@ -35,6 +36,7 @@ require ( golang.org/x/net v0.0.0-20181217023233-e147a9138326 // indirect google.golang.org/appengine v1.4.0 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect + gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect gopkg.in/go-playground/validator.v9 v9.21.0 gopkg.in/mail.v2 v2.3.1 ) diff --git a/go.sum b/go.sum index 447f1f5e1..b1f37dee7 100644 --- a/go.sum +++ b/go.sum @@ -53,6 +53,11 @@ github.com/jmoiron/sqlx v1.2.0 h1:41Ip0zITnmWNR/vHV+S4m+VoUivnWY5E4OJfLZjCJMA= github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks= github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= +github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= +github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 h1:MtvEpTB6LX3vkb4ax0b5D2DHbNAUsen0Gx5wZoq3lV4= github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k= github.com/lib/pq v1.0.0 h1:X5PMW56eZitiTeO7tKzZxFCSpbFZJtkMMooicw2us9A= @@ -123,6 +128,8 @@ gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc h1:2gG gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc/go.mod h1:m7x9LTH6d71AHyAX77c9yqWCCa3UKHcVEj9y7hAtKDk= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/go-playground/assert.v1 v1.2.1 h1:xoYuJVE7KT85PYWrN730RguIQO0ePzVRfFMXadIrXTM= gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE= gopkg.in/go-playground/validator.v9 v9.12.0/go.mod h1:+c9/zcJMFNgbLvly1L1V+PpxWdVbfP1avr/N00E2vyQ= diff --git a/mailroom.go b/mailroom.go index d0dbac4dc..8986d9f71 100644 --- a/mailroom.go +++ b/mailroom.go @@ -211,7 +211,7 @@ func (mr *Mailroom) Start() error { mr.handlerForeman.Start() // start our web server - mr.webserver = web.NewServer(mr.CTX, mr.Config, mr.DB, mr.RP, mr.S3Client, mr.WaitGroup) + mr.webserver = web.NewServer(mr.CTX, mr.Config, mr.DB, mr.RP, mr.S3Client, mr.ElasticClient, mr.WaitGroup) mr.webserver.Start() logrus.Info("mailroom started") diff --git a/models/contacts.go b/models/contacts.go index fed7b7a7b..1a2ca539f 100644 --- a/models/contacts.go +++ b/models/contacts.go @@ -148,6 +148,61 @@ func ContactIDsFromReferences(ctx context.Context, tx Queryer, org *OrgAssets, r return ids, nil } +// ContactIDsForQueryPage returns the ids of the contacts for the passed in query page +func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *OrgAssets, group assets.GroupUUID, query string, offset int, pageSize int) ([]ContactID, int64, error) { + start := time.Now() + + if client == nil { + return nil, 0, errors.Errorf("no elastic client available, check your configuration") + } + + // our field resolver + resolver := func(key string) assets.Field { + return org.FieldByKey(key) + } + + // turn into elastic query + eq, err := search.ToElasticQuery(org.Env(), resolver, query) + if err != nil { + return nil, 0, errors.Wrapf(err, "error converting contactql to elastic query: %s", query) + } + + eq = elastic.NewBoolQuery().Must( + eq, + elastic.NewTermQuery("org_id", org.OrgID()), + elastic.NewTermQuery("is_active", true), // technically this ought to be redundant with the group, but better to be safe + elastic.NewTermQuery("groups", group), + ) + + s := client.Search("contacts").Routing(strconv.FormatInt(int64(org.OrgID()), 10)) + s = s.Size(pageSize).From(offset).Query(eq).FetchSource(false) + + results, err := s.Do(ctx) + if err != nil { + return nil, 0, errors.Wrapf(err, "error performing query") + } + + ids := make([]ContactID, 0, pageSize) + for _, hit := range results.Hits.Hits { + id, err := strconv.Atoi(hit.Id) + if err != nil { + return nil, 0, errors.Wrapf(err, "unexpected non-integer contact id: %s for search: %s", hit.Id, query) + } + + ids = append(ids, ContactID(id)) + } + + logrus.WithFields(logrus.Fields{ + "org_id": org.OrgID(), + "group_uuid": group, + "query": query, + "elapsed": time.Since(start), + "match_count": len(ids), + }).Debug("paged contact query complete") + + return ids, results.Hits.TotalHits, nil +} + // ContactIDsForQuery returns the ids of all the contacts that match the passed in query func ContactIDsForQuery(ctx context.Context, client *elastic.Client, org *OrgAssets, query string) ([]ContactID, error) { start := time.Now() diff --git a/web/search/search.go b/web/search/search.go new file mode 100644 index 000000000..9a8d70a09 --- /dev/null +++ b/web/search/search.go @@ -0,0 +1,69 @@ +package search + +import ( + "context" + "net/http" + + "github.com/nyaruka/goflow/assets" + "github.com/nyaruka/goflow/utils" + "github.com/nyaruka/mailroom/models" + "github.com/nyaruka/mailroom/web" + "github.com/pkg/errors" +) + +func init() { + web.RegisterJSONRoute(http.MethodPost, "/mr/search/search", web.RequireAuthToken(handleSearch)) +} + +// Searches the contacts for an org +// +// { +// "org_id": 1, +// "group_uuid": "", +// "search": "age > 10" +// } +// +type searchRequest struct { + OrgID models.OrgID `json:"org_id" validate:"required"` + GroupUUID assets.GroupUUID `json:"group_uuid" validate:"required"` + Query string `json:"query" validate:"required"` + PageSize int `json:"page_size"` + Offset int `json:"offset"` +} + +// Response for a contact search +type searchResponse struct { + Parsed string `json:"parsed"` + Error string `json:"error"` + Results []models.ContactID `json:"results"` + Total int64 `json:"total"` + Offset int `json:"offset"` +} + +// handles a a contact search request +func handleSearch(ctx context.Context, s *web.Server, r *http.Request) (interface{}, int, error) { + request := &searchRequest{} + if err := utils.UnmarshalAndValidateWithLimit(r.Body, request, web.MaxRequestBytes); err != nil { + return nil, http.StatusBadRequest, errors.Wrapf(err, "request failed validation") + } + + // grab our org + org, err := models.NewOrgAssets(s.CTX, s.DB, request.OrgID, nil) + if err != nil { + return nil, http.StatusBadRequest, errors.Wrapf(err, "unable to load org assets") + } + + // Perform our search + hits, total, err := models.ContactIDsForQueryPage(ctx, s.ElasticClient, org, request.GroupUUID, request.Query, request.Offset, request.PageSize) + if err != nil { + return nil, http.StatusServiceUnavailable, errors.Wrapf(err, "error performing query") + } + + response := &searchResponse{ + Results: hits, + Total: total, + Offset: request.Offset, + } + + return response, http.StatusOK, nil +} diff --git a/web/server.go b/web/server.go index d0dadfead..d0bd19467 100644 --- a/web/server.go +++ b/web/server.go @@ -11,14 +11,15 @@ import ( "github.com/nyaruka/mailroom/config" "github.com/nyaruka/mailroom/models" + "github.com/olivere/elastic" - "github.com/pkg/errors" + "github.com/aws/aws-sdk-go/service/s3/s3iface" "github.com/go-chi/chi" "github.com/go-chi/chi/middleware" - "github.com/sirupsen/logrus" - "github.com/aws/aws-sdk-go/service/s3/s3iface" "github.com/gomodule/redigo/redis" "github.com/jmoiron/sqlx" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) const ( @@ -60,13 +61,14 @@ func RegisterRoute(method string, pattern string, handler Handler) { } // NewServer creates a new web server, it will need to be started after being created -func NewServer(ctx context.Context, config *config.Config, db *sqlx.DB, rp *redis.Pool, s3Client s3iface.S3API, wg *sync.WaitGroup) *Server { +func NewServer(ctx context.Context, config *config.Config, db *sqlx.DB, rp *redis.Pool, s3Client s3iface.S3API, elasticClient *elastic.Client, wg *sync.WaitGroup) *Server { s := &Server{ - CTX: ctx, - RP: rp, - DB: db, - S3Client: s3Client, - Config: config, + CTX: ctx, + RP: rp, + DB: db, + S3Client: s3Client, + ElasticClient: elasticClient, + Config: config, wg: wg, } @@ -271,11 +273,12 @@ func handle405(ctx context.Context, s *Server, r *http.Request) (interface{}, in } type Server struct { - CTX context.Context - RP *redis.Pool - DB *sqlx.DB - S3Client s3iface.S3API - Config *config.Config + CTX context.Context + RP *redis.Pool + DB *sqlx.DB + S3Client s3iface.S3API + Config *config.Config + ElasticClient *elastic.Client wg *sync.WaitGroup From dbf277dd6c542c7a54af3a7470a74363164210f2 Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Fri, 13 Dec 2019 17:21:23 -0800 Subject: [PATCH 02/24] sign our response --- web/search/search.go | 1 + 1 file changed, 1 insertion(+) diff --git a/web/search/search.go b/web/search/search.go index 9a8d70a09..feb1b40e0 100644 --- a/web/search/search.go +++ b/web/search/search.go @@ -59,6 +59,7 @@ func handleSearch(ctx context.Context, s *web.Server, r *http.Request) (interfac return nil, http.StatusServiceUnavailable, errors.Wrapf(err, "error performing query") } + // build our response response := &searchResponse{ Results: hits, Total: total, From d6dfab65f58e0ce1596ebaa66590ad3a63ea2bf3 Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Mon, 16 Dec 2019 17:02:34 -0800 Subject: [PATCH 03/24] sow for searching API, add unit test --- README.md | 2 +- models/contacts.go | 56 +++++---- models/test_constants.go | 3 + search/search.go | 13 +- search/search_test.go | 2 +- web/{search/search.go => contact/contact.go} | 25 ++-- web/contact/contact_test.go | 123 +++++++++++++++++++ 7 files changed, 183 insertions(+), 41 deletions(-) rename web/{search/search.go => contact/contact.go} (68%) create mode 100644 web/contact/contact_test.go diff --git a/README.md b/README.md index 88e944c8e..1776d7a08 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ To run the tests you need to create the test database: ``` $ createdb mailroom_test -$ createuser -P -E temba (set no password) +$ createuser -P -E -s mailroom_test (set no password) ``` To run all of the tests: diff --git a/models/contacts.go b/models/contacts.go index 1a2ca539f..cbc849e28 100644 --- a/models/contacts.go +++ b/models/contacts.go @@ -148,29 +148,45 @@ func ContactIDsFromReferences(ctx context.Context, tx Queryer, org *OrgAssets, r return ids, nil } -// ContactIDsForQueryPage returns the ids of the contacts for the passed in query page -func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *OrgAssets, group assets.GroupUUID, query string, offset int, pageSize int) ([]ContactID, int64, error) { - start := time.Now() - - if client == nil { - return nil, 0, errors.Errorf("no elastic client available, check your configuration") - } - +// ParseQuery parses the passed in query for the passed in org, returning the parsed query and the resulting elastic query +func ParseQuery(org *OrgAssets, query string) (string, elastic.Query, error) { // our field resolver resolver := func(key string) assets.Field { return org.FieldByKey(key) } // turn into elastic query - eq, err := search.ToElasticQuery(org.Env(), resolver, query) + parsed, eq, err := search.ToElasticQuery(org.Env(), resolver, query) if err != nil { - return nil, 0, errors.Wrapf(err, "error converting contactql to elastic query: %s", query) + return "", nil, errors.Wrapf(err, "error converting contactql to elastic query: %s", query) } + // additionally filter by org and active contacts eq = elastic.NewBoolQuery().Must( eq, elastic.NewTermQuery("org_id", org.OrgID()), - elastic.NewTermQuery("is_active", true), // technically this ought to be redundant with the group, but better to be safe + elastic.NewTermQuery("is_active", true), + ) + + return parsed, eq, nil +} + +// ContactIDsForQueryPage returns the ids of the contacts for the passed in query page +func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *OrgAssets, group assets.GroupUUID, query string, offset int, pageSize int) (string, []ContactID, int64, error) { + start := time.Now() + + if client == nil { + return "", nil, 0, errors.Errorf("no elastic client available, check your configuration") + } + + parsed, eq, err := ParseQuery(org, query) + if err != nil { + return "", nil, 0, errors.Wrapf(err, "error parsing query: %s", query) + } + + // filter by our base group + eq = elastic.NewBoolQuery().Must( + eq, elastic.NewTermQuery("groups", group), ) @@ -179,14 +195,14 @@ func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *Or results, err := s.Do(ctx) if err != nil { - return nil, 0, errors.Wrapf(err, "error performing query") + return "", nil, 0, errors.Wrapf(err, "error performing query") } ids := make([]ContactID, 0, pageSize) for _, hit := range results.Hits.Hits { id, err := strconv.Atoi(hit.Id) if err != nil { - return nil, 0, errors.Wrapf(err, "unexpected non-integer contact id: %s for search: %s", hit.Id, query) + return "", nil, 0, errors.Wrapf(err, "unexpected non-integer contact id: %s for search: %s", hit.Id, query) } ids = append(ids, ContactID(id)) @@ -194,13 +210,14 @@ func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *Or logrus.WithFields(logrus.Fields{ "org_id": org.OrgID(), + "parsed": parsed, "group_uuid": group, "query": query, "elapsed": time.Since(start), "match_count": len(ids), }).Debug("paged contact query complete") - return ids, results.Hits.TotalHits, nil + return parsed, ids, results.Hits.TotalHits, nil } // ContactIDsForQuery returns the ids of all the contacts that match the passed in query @@ -211,22 +228,15 @@ func ContactIDsForQuery(ctx context.Context, client *elastic.Client, org *OrgAss return nil, errors.Errorf("no elastic client available, check your configuration") } - // our field resolver - resolver := func(key string) assets.Field { - return org.FieldByKey(key) - } - // turn into elastic query - eq, err := search.ToElasticQuery(org.Env(), resolver, query) + _, eq, err := ParseQuery(org, query) if err != nil { return nil, errors.Wrapf(err, "error converting contactql to elastic query: %s", query) } - // filter by org, active, blocked and stopped + // only include unblocked and unstopped contacts eq = elastic.NewBoolQuery().Must( eq, - elastic.NewTermQuery("org_id", org.OrgID()), - elastic.NewTermQuery("is_active", true), elastic.NewTermQuery("is_blocked", false), elastic.NewTermQuery("is_stopped", false), ) diff --git a/models/test_constants.go b/models/test_constants.go index 01a9e2f2e..e29904254 100644 --- a/models/test_constants.go +++ b/models/test_constants.go @@ -80,6 +80,9 @@ var RemindersEvent2ID = CampaignEventID(10001) var DoctorsGroupID = GroupID(10000) var DoctorsGroupUUID = assets.GroupUUID("c153e265-f7c9-4539-9dbc-9b358714b638") +var AllContactsGroupID = GroupID(1) +var AllContactsGroupUUID = assets.GroupUUID("bc268217-9ffa-49e0-883e-e4e09c252a5a") + var TestersGroupID = GroupID(10001) var TestersGroupUUID = assets.GroupUUID("5e9d8fab-5e7e-4f51-b533-261af5dea70d") diff --git a/search/search.go b/search/search.go index 437da80a9..6fe187ce1 100644 --- a/search/search.go +++ b/search/search.go @@ -13,14 +13,19 @@ import ( "github.com/shopspring/decimal" ) -// ToElasticQuery converts a contactql query string to an Elastic query -func ToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, query string) (elastic.Query, error) { +// ToElasticQuery converts a contactql query string to an Elastic query returning the normalized view as well as the elastic query +func ToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, query string) (string, elastic.Query, error) { node, err := contactql.ParseQuery(query, env.RedactionPolicy(), resolver) if err != nil { - return nil, errors.Wrapf(err, "error parsing query: %s", query) + return "", nil, errors.Wrapf(err, "error parsing query: %s", query) } - return nodeToElasticQuery(env, resolver, node.Root()) + eq, err := nodeToElasticQuery(env, resolver, node.Root()) + if err != nil { + return "", nil, errors.Wrapf(err, "error parsing query: %s", query) + } + + return node.String(), eq, nil } func nodeToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, node contactql.QueryNode) (elastic.Query, error) { diff --git a/search/search_test.go b/search/search_test.go index 94047424c..cfbf6e22b 100644 --- a/search/search_test.go +++ b/search/search_test.go @@ -61,7 +61,7 @@ func TestElasticQuery(t *testing.T) { } env := envs.NewBuilder().WithTimezone(ny).WithRedactionPolicy(redactionPolicy).Build() - query, err := ToElasticQuery(env, resolver, tc.Search) + _, query, err := ToElasticQuery(env, resolver, tc.Search) if tc.Error != "" { assert.Error(t, err, "%s: error not received converting to elastic: %s", tc.Label, tc.Search) diff --git a/web/search/search.go b/web/contact/contact.go similarity index 68% rename from web/search/search.go rename to web/contact/contact.go index feb1b40e0..bf8c7794e 100644 --- a/web/search/search.go +++ b/web/contact/contact.go @@ -1,4 +1,4 @@ -package search +package contact import ( "context" @@ -12,7 +12,7 @@ import ( ) func init() { - web.RegisterJSONRoute(http.MethodPost, "/mr/search/search", web.RequireAuthToken(handleSearch)) + web.RegisterJSONRoute(http.MethodPost, "/mr/contact/search", web.RequireAuthToken(handleSearch)) } // Searches the contacts for an org @@ -33,11 +33,11 @@ type searchRequest struct { // Response for a contact search type searchResponse struct { - Parsed string `json:"parsed"` - Error string `json:"error"` - Results []models.ContactID `json:"results"` - Total int64 `json:"total"` - Offset int `json:"offset"` + Query string `json:"query"` + Error string `json:"error"` + ContactIDs []models.ContactID `json:"contact_ids"` + Total int64 `json:"total"` + Offset int `json:"offset"` } // handles a a contact search request @@ -48,22 +48,23 @@ func handleSearch(ctx context.Context, s *web.Server, r *http.Request) (interfac } // grab our org - org, err := models.NewOrgAssets(s.CTX, s.DB, request.OrgID, nil) + org, err := models.GetOrgAssets(s.CTX, s.DB, request.OrgID) if err != nil { return nil, http.StatusBadRequest, errors.Wrapf(err, "unable to load org assets") } // Perform our search - hits, total, err := models.ContactIDsForQueryPage(ctx, s.ElasticClient, org, request.GroupUUID, request.Query, request.Offset, request.PageSize) + parsed, hits, total, err := models.ContactIDsForQueryPage(ctx, s.ElasticClient, org, request.GroupUUID, request.Query, request.Offset, request.PageSize) if err != nil { return nil, http.StatusServiceUnavailable, errors.Wrapf(err, "error performing query") } // build our response response := &searchResponse{ - Results: hits, - Total: total, - Offset: request.Offset, + ContactIDs: hits, + Total: total, + Offset: request.Offset, + Query: parsed, } return response, http.StatusOK, nil diff --git a/web/contact/contact_test.go b/web/contact/contact_test.go new file mode 100644 index 000000000..687d425c2 --- /dev/null +++ b/web/contact/contact_test.go @@ -0,0 +1,123 @@ +package contact + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "io/ioutil" + "net/http" + "strings" + "sync" + "testing" + "time" + + "github.com/nyaruka/mailroom/config" + "github.com/nyaruka/mailroom/models" + "github.com/nyaruka/mailroom/search" + "github.com/nyaruka/mailroom/testsuite" + "github.com/nyaruka/mailroom/web" + "github.com/olivere/elastic" + "github.com/stretchr/testify/assert" +) + +func TestServer(t *testing.T) { + testsuite.Reset() + ctx := testsuite.CTX() + db := testsuite.DB() + rp := testsuite.RP() + wg := &sync.WaitGroup{} + + es := search.NewMockElasticServer() + defer es.Close() + + client, err := elastic.NewClient( + elastic.SetURL(es.URL()), + elastic.SetHealthcheck(false), + elastic.SetSniff(false), + ) + assert.NoError(t, err) + + server := web.NewServer(ctx, config.Mailroom, db, rp, nil, client, wg) + server.Start() + + // give our server time to start + time.Sleep(time.Second) + + defer server.Stop() + + tcs := []struct { + URL string + Method string + Body string + Status int + Response string + Hits []models.ContactID + ESResponse string + }{ + {"/mr/contact/search", "GET", "", 405, "illegal", nil, ""}, + { + "/mr/contact/search", "POST", + fmt.Sprintf(`{"org_id": 1, "query": "Cathy", "group_uuid": "%s"}`, models.AllContactsGroupUUID), + 200, + `name~Cathy`, + []models.ContactID{models.CathyID}, + fmt.Sprintf(`{ + "_scroll_id": "DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAbgc0WS1hqbHlfb01SM2lLTWJRMnVOSVZDdw==", + "took": 2, + "timed_out": false, + "_shards": { + "total": 1, + "successful": 1, + "skipped": 0, + "failed": 0 + }, + "hits": { + "total": 1, + "max_score": null, + "hits": [ + { + "_index": "contacts", + "_type": "_doc", + "_id": "%d", + "_score": null, + "_routing": "1", + "sort": [ + 15124352 + ] + } + ] + } + }`, models.CathyID), + }, + } + + for i, tc := range tcs { + var body io.Reader + es.NextResponse = tc.ESResponse + + if tc.Body != "" { + body = bytes.NewReader([]byte(tc.Body)) + } + + req, err := http.NewRequest(tc.Method, "http://localhost:8090"+tc.URL, body) + assert.NoError(t, err, "%d: error creating request", i) + + resp, err := http.DefaultClient.Do(req) + assert.NoError(t, err, "%d: error making request", i) + + assert.Equal(t, tc.Status, resp.StatusCode, "%d: unexpected status", i) + + content, err := ioutil.ReadAll(resp.Body) + assert.NoError(t, err, "%d: error reading body", i) + assert.True(t, strings.Contains(string(content), tc.Response), "%d: did not find string: %s in body: %s", i, tc.Response, string(content)) + + // on 200 responses parse them + if resp.StatusCode == 200 { + r := &searchResponse{} + err = json.Unmarshal(content, r) + assert.NoError(t, err) + assert.Equal(t, tc.Hits, r.ContactIDs) + } + } +} From 1dc5b1b72c2ef4cf18ad6be08fa518f3a5e1d643 Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Thu, 9 Jan 2020 11:32:28 -0800 Subject: [PATCH 04/24] update tests --- models/contacts_test.go | 30 +++++++++++++++++++++--------- web/expression/expression_test.go | 2 +- web/flow/flow_test.go | 2 +- web/ivr/ivr_test.go | 4 ++-- web/server_test.go | 2 +- web/simulation/simulation_test.go | 2 +- web/surveyor/surveyor_test.go | 4 ++-- 7 files changed, 29 insertions(+), 17 deletions(-) diff --git a/models/contacts_test.go b/models/contacts_test.go index dbbac76e7..7e0f43195 100644 --- a/models/contacts_test.go +++ b/models/contacts_test.go @@ -46,11 +46,19 @@ func TestElasticContacts(t *testing.T) { "query":{ "bool":{ "must":[ - {"match":{"name":{"query":"george"}}}, - {"term":{"org_id":1}}, - {"term":{"is_active":true}}, - {"term":{"is_blocked":false}}, - {"term":{"is_stopped":false}} + { "bool":{ + "must":[ + {"match":{"name":{"query":"george"}}}, + {"term":{"org_id":1}}, + {"term":{"is_active":true}} + ] + }}, + { "term":{ + "is_blocked":false + }}, + {"term": + {"is_stopped":false + }} ] } }, @@ -91,9 +99,13 @@ func TestElasticContacts(t *testing.T) { "query":{ "bool":{ "must":[ - {"match":{"name":{"query":"nobody"}}}, - {"term":{"org_id":1}}, - {"term":{"is_active":true}}, + {"bool": + {"must":[ + {"match":{"name":{"query":"nobody"}}}, + {"term":{"org_id":1}}, + {"term":{"is_active":true}} + ]} + }, {"term":{"is_blocked":false}}, {"term":{"is_stopped":false}} ] @@ -133,7 +145,7 @@ func TestElasticContacts(t *testing.T) { assert.Error(t, err) } else { assert.NoError(t, err, "%d: error encountered performing query", i) - assert.JSONEq(t, tc.Request, es.LastBody, "%d: request mismatch", i) + assert.JSONEq(t, tc.Request, es.LastBody, "%d: request mismatch, got: %s", i, es.LastBody) assert.Equal(t, tc.Contacts, ids, "%d: ids mismatch", i) } } diff --git a/web/expression/expression_test.go b/web/expression/expression_test.go index 00b1a93fd..0465112dd 100644 --- a/web/expression/expression_test.go +++ b/web/expression/expression_test.go @@ -26,7 +26,7 @@ func TestServer(t *testing.T) { rp := testsuite.RP() wg := &sync.WaitGroup{} - server := web.NewServer(ctx, config.Mailroom, db, rp, nil, wg) + server := web.NewServer(ctx, config.Mailroom, db, rp, nil, nil, wg) server.Start() // give our server time to start diff --git a/web/flow/flow_test.go b/web/flow/flow_test.go index 7e4bb384d..ea3f7b1bd 100644 --- a/web/flow/flow_test.go +++ b/web/flow/flow_test.go @@ -27,7 +27,7 @@ func TestServer(t *testing.T) { rp := testsuite.RP() wg := &sync.WaitGroup{} - server := web.NewServer(ctx, config.Mailroom, db, rp, nil, wg) + server := web.NewServer(ctx, config.Mailroom, db, rp, nil, nil, wg) server.Start() // give our server time to start diff --git a/web/ivr/ivr_test.go b/web/ivr/ivr_test.go index c7d535694..db0bc0f98 100644 --- a/web/ivr/ivr_test.go +++ b/web/ivr/ivr_test.go @@ -60,7 +60,7 @@ func TestTwilioIVR(t *testing.T) { twiml.IgnoreSignatures = true wg := &sync.WaitGroup{} - server := web.NewServer(ctx, config.Mailroom, db, rp, nil, wg) + server := web.NewServer(ctx, config.Mailroom, db, rp, nil, nil, wg) server.Start() defer server.Stop() @@ -364,7 +364,7 @@ func TestNexmoIVR(t *testing.T) { defer ts.Close() wg := &sync.WaitGroup{} - server := web.NewServer(ctx, config.Mailroom, db, rp, nil, wg) + server := web.NewServer(ctx, config.Mailroom, db, rp, nil, nil, wg) server.Start() defer server.Stop() diff --git a/web/server_test.go b/web/server_test.go index 723b66764..c75df80e3 100644 --- a/web/server_test.go +++ b/web/server_test.go @@ -23,7 +23,7 @@ func TestServer(t *testing.T) { rp := testsuite.RP() wg := &sync.WaitGroup{} - server := NewServer(ctx, config.Mailroom, db, rp, nil, wg) + server := NewServer(ctx, config.Mailroom, db, rp, nil, nil, wg) server.Start() // give our server time to start diff --git a/web/simulation/simulation_test.go b/web/simulation/simulation_test.go index 43caf3f39..bda6cc0a2 100644 --- a/web/simulation/simulation_test.go +++ b/web/simulation/simulation_test.go @@ -256,7 +256,7 @@ func TestServer(t *testing.T) { rp := testsuite.RP() wg := &sync.WaitGroup{} - server := web.NewServer(ctx, config.Mailroom, db, rp, nil, wg) + server := web.NewServer(ctx, config.Mailroom, db, rp, nil, nil, wg) server.Start() // give our server time to start diff --git a/web/surveyor/surveyor_test.go b/web/surveyor/surveyor_test.go index 5648657d8..f7558c1b4 100644 --- a/web/surveyor/surveyor_test.go +++ b/web/surveyor/surveyor_test.go @@ -9,8 +9,8 @@ import ( "sync" "testing" - "github.com/nyaruka/goflow/flows" "github.com/nyaruka/goflow/assets" + "github.com/nyaruka/goflow/flows" "github.com/nyaruka/mailroom/config" "github.com/nyaruka/mailroom/models" "github.com/nyaruka/mailroom/testsuite" @@ -27,7 +27,7 @@ func TestSurveyor(t *testing.T) { defer rc.Close() wg := &sync.WaitGroup{} - server := web.NewServer(ctx, config.Mailroom, db, rp, nil, wg) + server := web.NewServer(ctx, config.Mailroom, db, rp, nil, nil, wg) server.Start() defer server.Stop() From 2451722c2215669dd292c5d7479c5e75c20adcb4 Mon Sep 17 00:00:00 2001 From: Allan Lima Date: Fri, 10 Jan 2020 10:37:02 -0300 Subject: [PATCH 05/24] Add config option to max bytes of a webhook call response body --- config/config.go | 2 ++ goflow/engine.go | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index b574d8806..5212af12d 100644 --- a/config/config.go +++ b/config/config.go @@ -24,6 +24,7 @@ type Config struct { WebhooksTimeout int `help:"the timeout in milliseconds for webhook calls from engine"` WebhooksMaxRetries int `help:"the number of times to retry a failed webhook call"` + WebhooksMaxBodyBytes int `help:"the maximum size of bytes to a webhook call response body"` WebhooksInitialBackoff int `help:"the initial backoff in milliseconds when retrying a failed webhook call"` WebhooksBackoffJitter float64 `help:"the amount of jitter to apply to backoff times"` SMTPServer string `help:"the smtp configuration for sending emails ex: smtp://user%40password@server:port/?from=foo%40gmail.com"` @@ -66,6 +67,7 @@ func NewMailroomConfig() *Config { WebhooksTimeout: 15000, WebhooksMaxRetries: 2, + WebhooksMaxBodyBytes: 10000, WebhooksInitialBackoff: 5000, WebhooksBackoffJitter: 0.5, SMTPServer: "", diff --git a/goflow/engine.go b/goflow/engine.go index 5fe8f8f94..8117af6e5 100644 --- a/goflow/engine.go +++ b/goflow/engine.go @@ -55,7 +55,7 @@ func Engine() flows.Engine { httpClient, httpRetries := webhooksHTTP() eng = engine.NewBuilder(). - WithWebhookServiceFactory(webhooks.NewServiceFactory(httpClient, httpRetries, webhookHeaders, 10000)). + WithWebhookServiceFactory(webhooks.NewServiceFactory(httpClient, httpRetries, webhookHeaders, config.Mailroom.WebhooksMaxBodyBytes)). WithEmailServiceFactory(emailFactory). WithClassificationServiceFactory(classificationFactory). WithAirtimeServiceFactory(airtimeFactory). @@ -77,7 +77,7 @@ func Simulator() flows.Engine { httpClient, _ := webhooksHTTP() // don't do retries in simulator simulator = engine.NewBuilder(). - WithWebhookServiceFactory(webhooks.NewServiceFactory(httpClient, nil, webhookHeaders, 10000)). + WithWebhookServiceFactory(webhooks.NewServiceFactory(httpClient, nil, webhookHeaders, config.Mailroom.WebhooksMaxBodyBytes)). WithClassificationServiceFactory(classificationFactory). // simulated sessions do real classification WithEmailServiceFactory(simulatorEmailServiceFactory). // but faked emails WithAirtimeServiceFactory(simulatorAirtimeServiceFactory). // and faked airtime transfers From 5d086560026bd503597548121b8e3e55e0b629d0 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Mon, 13 Jan 2020 14:27:01 -0500 Subject: [PATCH 06/24] Include evaluation context with simulation requests --- go.mod | 2 +- go.sum | 4 ++-- web/simulation/simulation.go | 22 ++++++++++++++++------ web/simulation/simulation_test.go | 6 ++++++ 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 66ea4f7df..e508f941a 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/mattn/go-sqlite3 v1.10.0 // indirect github.com/nyaruka/ezconf v0.2.1 github.com/nyaruka/gocommon v1.1.1 - github.com/nyaruka/goflow v0.64.2 + github.com/nyaruka/goflow v0.64.5 github.com/nyaruka/librato v1.0.0 github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d github.com/nyaruka/null v1.2.0 diff --git a/go.sum b/go.sum index e92d03697..fed2e70e2 100644 --- a/go.sum +++ b/go.sum @@ -70,8 +70,8 @@ github.com/nyaruka/ezconf v0.2.1 h1:TDXWoqjqYya1uhou1mAJZg7rgFYL98EB0Tb3+BWtUh0= github.com/nyaruka/ezconf v0.2.1/go.mod h1:ey182kYkw2MIi4XiWe1FR/mzI33WCmTWuceDYYxgnQw= github.com/nyaruka/gocommon v1.1.1 h1:RnQ+kMzN1lA+W0NpkBDd0mGU3UqadJygR3SMpITMYTQ= github.com/nyaruka/gocommon v1.1.1/go.mod h1:QbdU2J9WBsqBmeZRuwndf2f6O7rD7mkC0bGn5UNnwjI= -github.com/nyaruka/goflow v0.64.2 h1:QFjEAqwFrzTBwqSWiW1o8NrS9wBRLz7Wt7Gcg5u5us4= -github.com/nyaruka/goflow v0.64.2/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= +github.com/nyaruka/goflow v0.64.5 h1:u0PGbLc5/R+mqL/DjcgHOiMXlIm2dRDBrF1pzWvOOJ0= +github.com/nyaruka/goflow v0.64.5/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= github.com/nyaruka/librato v1.0.0 h1:Vznj9WCeC1yZXbBYyYp40KnbmXLbEkjKmHesV/v2SR0= github.com/nyaruka/librato v1.0.0/go.mod h1:pkRNLFhFurOz0QqBz6/DuTFhHHxAubWxs4Jx+J7yUgg= github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d h1:hyp9u36KIwbTCo2JAJ+TuJcJBc+UZzEig7RI/S5Dvkc= diff --git a/web/simulation/simulation.go b/web/simulation/simulation.go index 38eb82526..88ca5ec98 100644 --- a/web/simulation/simulation.go +++ b/web/simulation/simulation.go @@ -8,6 +8,7 @@ import ( "github.com/jmoiron/sqlx" "github.com/nyaruka/goflow/assets" "github.com/nyaruka/goflow/assets/static/types" + xtypes "github.com/nyaruka/goflow/excellent/types" "github.com/nyaruka/goflow/flows" "github.com/nyaruka/goflow/flows/events" "github.com/nyaruka/goflow/flows/resumes" @@ -38,9 +39,18 @@ type sessionRequest struct { } `json:"assets"` } -type sessionResponse struct { - Session flows.Session `json:"session"` - Events []flows.Event `json:"events"` +type simulationResponse struct { + Session flows.Session `json:"session"` + Events []flows.Event `json:"events"` + Context map[string]xtypes.XValue `json:"context,omitempty"` +} + +func newSimulationResponse(session flows.Session, sprint flows.Sprint) *simulationResponse { + var context map[string]xtypes.XValue + if session != nil { + context = session.CurrentContext() + } + return &simulationResponse{Session: session, Events: sprint.Events(), Context: context} } // Starts a new engine session @@ -136,7 +146,7 @@ func triggerFlow(ctx context.Context, db *sqlx.DB, org *models.OrgAssets, sa flo return nil, http.StatusInternalServerError, errors.Wrapf(err, "error handling simulation events") } - return &sessionResponse{Session: session, Events: sprint.Events()}, http.StatusOK, nil + return newSimulationResponse(session, sprint), http.StatusOK, nil } // Resumes an existing engine session @@ -236,7 +246,7 @@ func handleResume(ctx context.Context, s *web.Server, r *http.Request) (interfac // if our session is already complete, then this is a no-op, return the session unchanged if session.Status() != flows.SessionStatusWaiting { - return &sessionResponse{Session: session, Events: nil}, http.StatusOK, nil + return &simulationResponse{Session: session, Events: nil}, http.StatusOK, nil } // resume our session @@ -250,7 +260,7 @@ func handleResume(ctx context.Context, s *web.Server, r *http.Request) (interfac return nil, http.StatusInternalServerError, errors.Wrapf(err, "error handling simulation events") } - return &sessionResponse{Session: session, Events: sprint.Events()}, http.StatusOK, nil + return newSimulationResponse(session, sprint), http.StatusOK, nil } // populateFlow takes care of setting the definition for the flow with the passed in UUID according to the passed in definitions diff --git a/web/simulation/simulation_test.go b/web/simulation/simulation_test.go index 43caf3f39..d58edde91 100644 --- a/web/simulation/simulation_test.go +++ b/web/simulation/simulation_test.go @@ -329,6 +329,12 @@ func TestServer(t *testing.T) { sessionJSON, _ := json.Marshal(parsed["session"]) session = string(sessionJSON) fmt.Println(session) + + context, hasContext := parsed["context"] + if hasContext { + assert.Contains(t, context, "contact") + assert.Contains(t, context, "globals") + } } assert.True(t, strings.Contains(string(content), tc.Response), "%d: did not find string: %s in body: %s", i, tc.Response, string(content)) From 9778d19be44bc3d43fb7d60206006d21c21f23ab Mon Sep 17 00:00:00 2001 From: John Cordeiro Date: Mon, 13 Jan 2020 20:17:50 -0300 Subject: [PATCH 07/24] Fix bothub classifier type --- models/classifiers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/classifiers.go b/models/classifiers.go index e2f5f324e..a87306862 100644 --- a/models/classifiers.go +++ b/models/classifiers.go @@ -29,7 +29,7 @@ const ( // Our classifier types ClassifierTypeWit = "wit" ClassifierTypeLuis = "luis" - ClassifierTypeBothub = "bh" + ClassifierTypeBothub = "bothub" // Wit.ai config options WitConfigAccessToken = "access_token" From c2ae27904dce7b203d25742ed5d058be3b1f1d92 Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Mon, 13 Jan 2020 18:18:15 -0800 Subject: [PATCH 08/24] add sorting support --- models/contacts.go | 26 +++++++++++++++------- search/search.go | 34 +++++++++++++++++++++++++++++ search/search_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++ web/contact/contact.go | 18 +++++++++++----- 4 files changed, 114 insertions(+), 13 deletions(-) diff --git a/models/contacts.go b/models/contacts.go index d57cee380..dc6a44e7f 100644 --- a/models/contacts.go +++ b/models/contacts.go @@ -11,6 +11,7 @@ import ( "github.com/nyaruka/gocommon/urns" "github.com/nyaruka/goflow/assets" + "github.com/nyaruka/goflow/contactql" "github.com/nyaruka/goflow/envs" "github.com/nyaruka/goflow/excellent/types" "github.com/nyaruka/goflow/flows" @@ -148,17 +149,19 @@ func ContactIDsFromReferences(ctx context.Context, tx Queryer, org *OrgAssets, r return ids, nil } -// ParseQuery parses the passed in query for the passed in org, returning the parsed query and the resulting elastic query -func ParseQuery(org *OrgAssets, query string) (string, elastic.Query, error) { - // our field resolver - resolver := func(key string) assets.Field { +// buildFieldResolver builds a field resolver function for the passed in Org +func buildFieldResolver(org *OrgAssets) contactql.FieldResolverFunc { + return func(key string) assets.Field { f := org.FieldByKey(key) if f == nil { return nil } return f } +} +// ParseQuery parses the passed in query for the passed in org, returning the parsed query and the resulting elastic query +func ParseQuery(org *OrgAssets, resolver contactql.FieldResolverFunc, query string) (string, elastic.Query, error) { // turn into elastic query parsed, eq, err := search.ToElasticQuery(org.Env(), resolver, query) if err != nil { @@ -176,18 +179,24 @@ func ParseQuery(org *OrgAssets, query string) (string, elastic.Query, error) { } // ContactIDsForQueryPage returns the ids of the contacts for the passed in query page -func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *OrgAssets, group assets.GroupUUID, query string, offset int, pageSize int) (string, []ContactID, int64, error) { +func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *OrgAssets, group assets.GroupUUID, query string, sort string, offset int, pageSize int) (string, []ContactID, int64, error) { start := time.Now() if client == nil { return "", nil, 0, errors.Errorf("no elastic client available, check your configuration") } - parsed, eq, err := ParseQuery(org, query) + resolver := buildFieldResolver(org) + parsed, eq, err := ParseQuery(org, resolver, query) if err != nil { return "", nil, 0, errors.Wrapf(err, "error parsing query: %s", query) } + fieldSort, err := search.ToElasticFieldSort(resolver, sort) + if err != nil { + return "", nil, 0, errors.Wrapf(err, "error parsing sort") + } + // filter by our base group eq = elastic.NewBoolQuery().Must( eq, @@ -195,7 +204,7 @@ func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *Or ) s := client.Search("contacts").Routing(strconv.FormatInt(int64(org.OrgID()), 10)) - s = s.Size(pageSize).From(offset).Query(eq).FetchSource(false) + s = s.Size(pageSize).From(offset).Query(eq).SortBy(fieldSort).FetchSource(false) results, err := s.Do(ctx) if err != nil { @@ -233,7 +242,8 @@ func ContactIDsForQuery(ctx context.Context, client *elastic.Client, org *OrgAss } // turn into elastic query - _, eq, err := ParseQuery(org, query) + resolver := buildFieldResolver(org) + _, eq, err := ParseQuery(org, resolver, query) if err != nil { return nil, errors.Wrapf(err, "error converting contactql to elastic query: %s", query) } diff --git a/search/search.go b/search/search.go index 6fe187ce1..a99769391 100644 --- a/search/search.go +++ b/search/search.go @@ -28,6 +28,40 @@ func ToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, return node.String(), eq, nil } +// ToElasticFieldSort returns the FieldSort for the passed in field +func ToElasticFieldSort(resolver contactql.FieldResolverFunc, fieldName string) (*elastic.FieldSort, error) { + // no field name? default to most recent first by id + if fieldName == "" { + return elastic.NewFieldSort("id").Desc(), nil + } + + // figure out if we are ascending or descending (default is ascending, can be changed with leading -) + ascending := true + if strings.HasPrefix(fieldName, "-") { + ascending = false + fieldName = fieldName[1:] + } + + fieldName = strings.ToLower(fieldName) + + // we are sorting by an attribute + if fieldName == contactql.AttributeID || fieldName == contactql.AttributeCreatedOn || + fieldName == contactql.AttributeLanguage || fieldName == contactql.AttributeName { + return elastic.NewFieldSort(fieldName).Order(ascending), nil + } + + // we are sorting by a custom field + field := resolver(fieldName) + if field == nil { + return nil, errors.Errorf("unable to find field with name: %s", fieldName) + } + + sort := elastic.NewFieldSort(fmt.Sprintf("fields.%s", field.Type())) + sort = sort.Nested(elastic.NewNestedSort("fields").Filter(elastic.NewTermQuery("fields.field", field.UUID()))) + sort = sort.Order(ascending) + return sort, nil +} + func nodeToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, node contactql.QueryNode) (elastic.Query, error) { switch n := node.(type) { case *contactql.BoolCombination: diff --git a/search/search_test.go b/search/search_test.go index cfbf6e22b..ea6afa3d9 100644 --- a/search/search_test.go +++ b/search/search_test.go @@ -3,6 +3,7 @@ package search import ( "bytes" "encoding/json" + "fmt" "io/ioutil" "testing" "time" @@ -23,6 +24,54 @@ func (f *MockField) Name() string { return f.fieldKey } func (f *MockField) Type() assets.FieldType { return f.fieldType } func (f *MockField) UUID() assets.FieldUUID { return f.fieldUUID } +func TestElasticSort(t *testing.T) { + registry := map[string]assets.Field{ + "age": &MockField{"age", assets.FieldTypeNumber, "6b6a43fa-a26d-4017-bede-328bcdd5c93b"}, + "color": &MockField{"color", assets.FieldTypeText, "ecc7b13b-c698-4f46-8a90-24a8fab6fe34"}, + "dob": &MockField{"dob", assets.FieldTypeDatetime, "cbd3fc0e-9b74-4207-a8c7-248082bb4572"}, + "state": &MockField{"state", assets.FieldTypeState, "67663ad1-3abc-42dd-a162-09df2dea66ec"}, + "district": &MockField{"district", assets.FieldTypeDistrict, "54c72635-d747-4e45-883c-099d57dd998e"}, + "ward": &MockField{"ward", assets.FieldTypeWard, "fde8f740-c337-421b-8abb-83b954897c80"}, + } + + tcs := []struct { + Label string + Sort string + Elastic string + Error error + }{ + {"empty", "", `{"id":{"order":"desc"}}`, nil}, + {"descending created_on", "-created_on", `{"created_on":{"order":"desc"}}`, nil}, + {"ascending name", "name", `{"name":{"order":"asc"}}`, nil}, + {"descending language", "-language", `{"language":{"order":"desc"}}`, nil}, + {"descending numeric", "-age", `{"fields.number":{"nested":{"filter":{"term":{"fields.field":"6b6a43fa-a26d-4017-bede-328bcdd5c93b"}},"path":"fields"},"order":"desc"}}`, nil}, + {"ascending text", "color", `{"fields.text":{"nested":{"filter":{"term":{"fields.field":"ecc7b13b-c698-4f46-8a90-24a8fab6fe34"}},"path":"fields"},"order":"asc"}}`, nil}, + {"descending date", "-dob", `{"fields.datetime":{"nested":{"filter":{"term":{"fields.field":"cbd3fc0e-9b74-4207-a8c7-248082bb4572"}},"path":"fields"},"order":"desc"}}`, nil}, + {"descending state", "-state", `{"fields.state":{"nested":{"filter":{"term":{"fields.field":"67663ad1-3abc-42dd-a162-09df2dea66ec"}},"path":"fields"},"order":"desc"}}`, nil}, + {"ascending district", "district", `{"fields.district":{"nested":{"filter":{"term":{"fields.field":"54c72635-d747-4e45-883c-099d57dd998e"}},"path":"fields"},"order":"asc"}}`, nil}, + {"ascending ward", "ward", `{"fields.ward":{"nested":{"filter":{"term":{"fields.field":"fde8f740-c337-421b-8abb-83b954897c80"}},"path":"fields"},"order":"asc"}}`, nil}, + + {"unknown field", "foo", "", fmt.Errorf("unable to find field with name: foo")}, + } + + resolver := func(key string) assets.Field { + return registry[key] + } + + for _, tc := range tcs { + sort, err := ToElasticFieldSort(resolver, tc.Sort) + + if err != nil { + assert.Equal(t, tc.Error.Error(), err.Error()) + continue + } + + src, _ := sort.Source() + encoded, _ := json.Marshal(src) + assert.Equal(t, tc.Elastic, string(encoded)) + } +} + func TestElasticQuery(t *testing.T) { registry := map[string]assets.Field{ "age": &MockField{"age", assets.FieldTypeNumber, "6b6a43fa-a26d-4017-bede-328bcdd5c93b"}, diff --git a/web/contact/contact.go b/web/contact/contact.go index bf8c7794e..8e1a797a9 100644 --- a/web/contact/contact.go +++ b/web/contact/contact.go @@ -19,8 +19,9 @@ func init() { // // { // "org_id": 1, -// "group_uuid": "", -// "search": "age > 10" +// "group_uuid": "985a83fe-2e9f-478d-a3ec-fa602d5e7ddd", +// "search": "age > 10", +// "sort": "-age" // } // type searchRequest struct { @@ -29,12 +30,19 @@ type searchRequest struct { Query string `json:"query" validate:"required"` PageSize int `json:"page_size"` Offset int `json:"offset"` + Sort string `json:"sort"` } // Response for a contact search +// +// { +// "query": "age > 10", +// "contact_ids": [5,10,15], +// "total": 3, +// "offset": 0 +// } type searchResponse struct { Query string `json:"query"` - Error string `json:"error"` ContactIDs []models.ContactID `json:"contact_ids"` Total int64 `json:"total"` Offset int `json:"offset"` @@ -54,17 +62,17 @@ func handleSearch(ctx context.Context, s *web.Server, r *http.Request) (interfac } // Perform our search - parsed, hits, total, err := models.ContactIDsForQueryPage(ctx, s.ElasticClient, org, request.GroupUUID, request.Query, request.Offset, request.PageSize) + parsed, hits, total, err := models.ContactIDsForQueryPage(ctx, s.ElasticClient, org, request.GroupUUID, request.Query, request.Sort, request.Offset, request.PageSize) if err != nil { return nil, http.StatusServiceUnavailable, errors.Wrapf(err, "error performing query") } // build our response response := &searchResponse{ + Query: parsed, ContactIDs: hits, Total: total, Offset: request.Offset, - Query: parsed, } return response, http.StatusOK, nil From 8fda91f136810c5e608f7eca5dcd6ef808f7442c Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Tue, 14 Jan 2020 13:42:06 -0800 Subject: [PATCH 09/24] refactors errors so we can get out parsing / field errors --- cmd/mailroom/main.go | 1 + models/contacts.go | 4 ++-- search/search.go | 49 ++++++++++++++++++++++++++---------------- web/contact/contact.go | 26 ++++++++++++++++------ 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/cmd/mailroom/main.go b/cmd/mailroom/main.go index 6db0cd3ce..ae5292de7 100644 --- a/cmd/mailroom/main.go +++ b/cmd/mailroom/main.go @@ -24,6 +24,7 @@ import ( _ "github.com/nyaruka/mailroom/tasks/stats" _ "github.com/nyaruka/mailroom/tasks/timeouts" + _ "github.com/nyaruka/mailroom/web/contact" _ "github.com/nyaruka/mailroom/web/docs" _ "github.com/nyaruka/mailroom/web/expression" _ "github.com/nyaruka/mailroom/web/flow" diff --git a/models/contacts.go b/models/contacts.go index dc6a44e7f..824837495 100644 --- a/models/contacts.go +++ b/models/contacts.go @@ -217,7 +217,6 @@ func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *Or if err != nil { return "", nil, 0, errors.Wrapf(err, "unexpected non-integer contact id: %s for search: %s", hit.Id, query) } - ids = append(ids, ContactID(id)) } @@ -227,7 +226,8 @@ func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *Or "group_uuid": group, "query": query, "elapsed": time.Since(start), - "match_count": len(ids), + "page_count": len(ids), + "total_count": results.Hits.TotalHits, }).Debug("paged contact query complete") return parsed, ids, results.Hits.TotalHits, nil diff --git a/search/search.go b/search/search.go index a99769391..395df2b9a 100644 --- a/search/search.go +++ b/search/search.go @@ -17,7 +17,7 @@ import ( func ToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, query string) (string, elastic.Query, error) { node, err := contactql.ParseQuery(query, env.RedactionPolicy(), resolver) if err != nil { - return "", nil, errors.Wrapf(err, "error parsing query: %s", query) + return "", nil, NewError("error parsing query: %s", err.Error()) } eq, err := nodeToElasticQuery(env, resolver, node.Root()) @@ -53,7 +53,7 @@ func ToElasticFieldSort(resolver contactql.FieldResolverFunc, fieldName string) // we are sorting by a custom field field := resolver(fieldName) if field == nil { - return nil, errors.Errorf("unable to find field with name: %s", fieldName) + return nil, NewError("unable to find field with name: %s", fieldName) } sort := elastic.NewFieldSort(fmt.Sprintf("fields.%s", field.Type())) @@ -97,7 +97,7 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol if c.PropertyType() == contactql.PropertyTypeField { field := resolver(key) if field == nil { - return nil, errors.Errorf("unable to find field: %s", key) + return nil, NewError("unable to find field: %s", key) } fieldQuery := elastic.NewTermQuery("fields.field", field.UUID()) @@ -129,7 +129,7 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol ) return elastic.NewBoolQuery().MustNot(elastic.NewNestedQuery("fields", query)), nil } else { - return nil, fmt.Errorf("unsupported text comparator: %s", c.Comparator()) + return nil, NewError("unsupported text comparator: %s", c.Comparator()) } return elastic.NewNestedQuery("fields", elastic.NewBoolQuery().Must(fieldQuery, query)), nil @@ -137,7 +137,7 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol } else if fieldType == assets.FieldTypeNumber { value, err := decimal.NewFromString(c.Value()) if err != nil { - return nil, errors.Errorf("can't convert '%s' to a number", c.Value()) + return nil, NewError("can't convert '%s' to a number", c.Value()) } if c.Comparator() == "=" { @@ -151,7 +151,7 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol } else if c.Comparator() == "<=" { query = elastic.NewRangeQuery("fields.number").Lte(value) } else { - return nil, fmt.Errorf("unsupported number comparator: %s", c.Comparator()) + return nil, NewError("unsupported number comparator: %s", c.Comparator()) } return elastic.NewNestedQuery("fields", elastic.NewBoolQuery().Must(fieldQuery, query)), nil @@ -159,7 +159,7 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol } else if fieldType == assets.FieldTypeDatetime { value, err := envs.DateTimeFromString(env, c.Value(), false) if err != nil { - return nil, errors.Wrapf(err, "unable to parse datetime: %s", c.Value()) + return nil, NewError("unable to parse datetime: %s", c.Value()) } start, end := dates.DayToUTCRange(value, value.Location()) @@ -174,7 +174,7 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol } else if c.Comparator() == "<=" { query = elastic.NewRangeQuery("fields.datetime").Lt(end) } else { - return nil, fmt.Errorf("unsupported datetime comparator: %s", c.Comparator()) + return nil, NewError("unsupported datetime comparator: %s", c.Comparator()) } return elastic.NewNestedQuery("fields", elastic.NewBoolQuery().Must(fieldQuery, query)), nil @@ -195,12 +195,12 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol ), ), nil } else { - return nil, fmt.Errorf("unsupported location comparator: %s", c.Comparator()) + return nil, NewError("unsupported location comparator: %s", c.Comparator()) } return elastic.NewNestedQuery("fields", elastic.NewBoolQuery().Must(fieldQuery, query)), nil } else { - return nil, fmt.Errorf("unsupported contact field type: %s", field.Type()) + return nil, NewError("unsupported contact field type: %s", field.Type()) } } else if c.PropertyType() == contactql.PropertyTypeAttribute { value := strings.ToLower(c.Value()) @@ -229,25 +229,25 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol } else if c.Comparator() == "!=" { return elastic.NewBoolQuery().MustNot(elastic.NewTermQuery("name.keyword", c.Value())), nil } else { - return nil, fmt.Errorf("unsupported name query comparator: %s", c.Comparator()) + return nil, NewError("unsupported name query comparator: %s", c.Comparator()) } } else if key == contactql.AttributeID { if c.Comparator() == "=" { return elastic.NewIdsQuery().Ids(value), nil } - return nil, fmt.Errorf("unsupported comparator for id: %s", c.Comparator()) + return nil, NewError("unsupported comparator for id: %s", c.Comparator()) } else if key == contactql.AttributeLanguage { if c.Comparator() == "=" { return elastic.NewTermQuery("language", value), nil } else if c.Comparator() == "!=" { return elastic.NewBoolQuery().MustNot(elastic.NewTermQuery("language", value)), nil } else { - return nil, fmt.Errorf("unsupported language comparator: %s", c.Comparator()) + return nil, NewError("unsupported language comparator: %s", c.Comparator()) } } else if key == contactql.AttributeCreatedOn { value, err := envs.DateTimeFromString(env, c.Value(), false) if err != nil { - return nil, errors.Wrapf(err, "unable to parse datetime: %s", c.Value()) + return nil, NewError("unable to parse datetime: %s", c.Value()) } start, end := dates.DayToUTCRange(value, value.Location()) @@ -262,10 +262,10 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol } else if c.Comparator() == "<=" { return elastic.NewRangeQuery("created_on").Lt(end), nil } else { - return nil, fmt.Errorf("unsupported created_on comparator: %s", c.Comparator()) + return nil, NewError("unsupported created_on comparator: %s", c.Comparator()) } } else { - return nil, fmt.Errorf("unsupported contact attribute: %s", key) + return nil, NewError("unsupported contact attribute: %s", key) } } else if c.PropertyType() == contactql.PropertyTypeScheme { value := strings.ToLower(c.Value()) @@ -293,9 +293,22 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol elastic.NewTermQuery("urns.scheme", key)), ), nil } else { - return nil, fmt.Errorf("unsupported scheme comparator: %s", c.Comparator()) + return nil, NewError("unsupported scheme comparator: %s", c.Comparator()) } } - return nil, errors.Errorf("unsupported property type: %s", c.PropertyType()) + return nil, NewError("unsupported property type: %s", c.PropertyType()) +} + +// Error is used when an error is in the parsing of a field or query format +type Error struct { + error string +} + +func (e *Error) Error() string { + return e.error +} + +func NewError(err string, args ...interface{}) *Error { + return &Error{fmt.Sprintf(err, args...)} } diff --git a/web/contact/contact.go b/web/contact/contact.go index 8e1a797a9..69ddce69d 100644 --- a/web/contact/contact.go +++ b/web/contact/contact.go @@ -7,6 +7,7 @@ import ( "github.com/nyaruka/goflow/assets" "github.com/nyaruka/goflow/utils" "github.com/nyaruka/mailroom/models" + "github.com/nyaruka/mailroom/search" "github.com/nyaruka/mailroom/web" "github.com/pkg/errors" ) @@ -20,7 +21,7 @@ func init() { // { // "org_id": 1, // "group_uuid": "985a83fe-2e9f-478d-a3ec-fa602d5e7ddd", -// "search": "age > 10", +// "query": "age > 10", // "sort": "-age" // } // @@ -46,25 +47,37 @@ type searchResponse struct { ContactIDs []models.ContactID `json:"contact_ids"` Total int64 `json:"total"` Offset int `json:"offset"` + Sort string `json:"sort"` } // handles a a contact search request func handleSearch(ctx context.Context, s *web.Server, r *http.Request) (interface{}, int, error) { - request := &searchRequest{} + request := &searchRequest{ + Offset: 0, + PageSize: 50, + Sort: "-created_on", + } if err := utils.UnmarshalAndValidateWithLimit(r.Body, request, web.MaxRequestBytes); err != nil { - return nil, http.StatusBadRequest, errors.Wrapf(err, "request failed validation") + return errors.Wrapf(err, "request failed validation"), http.StatusBadRequest, nil } // grab our org org, err := models.GetOrgAssets(s.CTX, s.DB, request.OrgID) if err != nil { - return nil, http.StatusBadRequest, errors.Wrapf(err, "unable to load org assets") + return nil, http.StatusInternalServerError, errors.Wrapf(err, "unable to load org assets") } // Perform our search - parsed, hits, total, err := models.ContactIDsForQueryPage(ctx, s.ElasticClient, org, request.GroupUUID, request.Query, request.Sort, request.Offset, request.PageSize) + parsed, hits, total, err := models.ContactIDsForQueryPage(ctx, s.ElasticClient, org, + request.GroupUUID, request.Query, request.Sort, request.Offset, request.PageSize) + if err != nil { - return nil, http.StatusServiceUnavailable, errors.Wrapf(err, "error performing query") + switch cause := errors.Cause(err).(type) { + case *search.Error: + return cause, http.StatusBadRequest, nil + default: + return nil, http.StatusInternalServerError, err + } } // build our response @@ -73,6 +86,7 @@ func handleSearch(ctx context.Context, s *web.Server, r *http.Request) (interfac ContactIDs: hits, Total: total, Offset: request.Offset, + Sort: request.Sort, } return response, http.StatusOK, nil From 3ed54415da66ead1f22035ac917d4ecbceda1554 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Wed, 15 Jan 2020 16:30:11 -0500 Subject: [PATCH 10/24] Simulation context should include defaults --- go.mod | 2 +- go.sum | 4 ++-- web/simulation/simulation.go | 16 ++++++++++++---- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index e508f941a..224864f0a 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/mattn/go-sqlite3 v1.10.0 // indirect github.com/nyaruka/ezconf v0.2.1 github.com/nyaruka/gocommon v1.1.1 - github.com/nyaruka/goflow v0.64.5 + github.com/nyaruka/goflow v0.64.6 github.com/nyaruka/librato v1.0.0 github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d github.com/nyaruka/null v1.2.0 diff --git a/go.sum b/go.sum index fed2e70e2..7f075fef3 100644 --- a/go.sum +++ b/go.sum @@ -70,8 +70,8 @@ github.com/nyaruka/ezconf v0.2.1 h1:TDXWoqjqYya1uhou1mAJZg7rgFYL98EB0Tb3+BWtUh0= github.com/nyaruka/ezconf v0.2.1/go.mod h1:ey182kYkw2MIi4XiWe1FR/mzI33WCmTWuceDYYxgnQw= github.com/nyaruka/gocommon v1.1.1 h1:RnQ+kMzN1lA+W0NpkBDd0mGU3UqadJygR3SMpITMYTQ= github.com/nyaruka/gocommon v1.1.1/go.mod h1:QbdU2J9WBsqBmeZRuwndf2f6O7rD7mkC0bGn5UNnwjI= -github.com/nyaruka/goflow v0.64.5 h1:u0PGbLc5/R+mqL/DjcgHOiMXlIm2dRDBrF1pzWvOOJ0= -github.com/nyaruka/goflow v0.64.5/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= +github.com/nyaruka/goflow v0.64.6 h1:maKHM2yVLeqyFhsU7z4whtFLnfACdWvJ/6V+rPeY8mg= +github.com/nyaruka/goflow v0.64.6/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= github.com/nyaruka/librato v1.0.0 h1:Vznj9WCeC1yZXbBYyYp40KnbmXLbEkjKmHesV/v2SR0= github.com/nyaruka/librato v1.0.0/go.mod h1:pkRNLFhFurOz0QqBz6/DuTFhHHxAubWxs4Jx+J7yUgg= github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d h1:hyp9u36KIwbTCo2JAJ+TuJcJBc+UZzEig7RI/S5Dvkc= diff --git a/web/simulation/simulation.go b/web/simulation/simulation.go index 88ca5ec98..e9f852cd5 100644 --- a/web/simulation/simulation.go +++ b/web/simulation/simulation.go @@ -8,6 +8,7 @@ import ( "github.com/jmoiron/sqlx" "github.com/nyaruka/goflow/assets" "github.com/nyaruka/goflow/assets/static/types" + "github.com/nyaruka/goflow/excellent/tools" xtypes "github.com/nyaruka/goflow/excellent/types" "github.com/nyaruka/goflow/flows" "github.com/nyaruka/goflow/flows/events" @@ -40,15 +41,22 @@ type sessionRequest struct { } type simulationResponse struct { - Session flows.Session `json:"session"` - Events []flows.Event `json:"events"` - Context map[string]xtypes.XValue `json:"context,omitempty"` + Session flows.Session `json:"session"` + Events []flows.Event `json:"events"` + Context *xtypes.XObject `json:"context,omitempty"` } func newSimulationResponse(session flows.Session, sprint flows.Sprint) *simulationResponse { - var context map[string]xtypes.XValue + var context *xtypes.XObject if session != nil { context = session.CurrentContext() + + // include object defaults which are not marshaled by default + if context != nil { + tools.ContextWalkObjects(context, func(o *xtypes.XObject) { + o.SetMarshalDefault(true) + }) + } } return &simulationResponse{Session: session, Events: sprint.Events(), Context: context} } From 93f5ef95ec3e334edf3a690aee7fb2664c108fc6 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 16 Jan 2020 09:21:19 -0500 Subject: [PATCH 11/24] Update CHANGELOG.md for v5.3.17 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2ab58991..d6f4a5de1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +v5.3.17 +---------- + * Include evaluation context with simulation requests + v5.3.16 ---------- * Update to goflow v0.64.2 From e8b6b2a6a19930e083f1c19b08caa1490aabbbe6 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 16 Jan 2020 13:24:01 -0500 Subject: [PATCH 12/24] Update to goflow v0.64.7 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 224864f0a..64d3b9f73 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/mattn/go-sqlite3 v1.10.0 // indirect github.com/nyaruka/ezconf v0.2.1 github.com/nyaruka/gocommon v1.1.1 - github.com/nyaruka/goflow v0.64.6 + github.com/nyaruka/goflow v0.64.7 github.com/nyaruka/librato v1.0.0 github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d github.com/nyaruka/null v1.2.0 diff --git a/go.sum b/go.sum index 7f075fef3..abf3be0ff 100644 --- a/go.sum +++ b/go.sum @@ -70,8 +70,8 @@ github.com/nyaruka/ezconf v0.2.1 h1:TDXWoqjqYya1uhou1mAJZg7rgFYL98EB0Tb3+BWtUh0= github.com/nyaruka/ezconf v0.2.1/go.mod h1:ey182kYkw2MIi4XiWe1FR/mzI33WCmTWuceDYYxgnQw= github.com/nyaruka/gocommon v1.1.1 h1:RnQ+kMzN1lA+W0NpkBDd0mGU3UqadJygR3SMpITMYTQ= github.com/nyaruka/gocommon v1.1.1/go.mod h1:QbdU2J9WBsqBmeZRuwndf2f6O7rD7mkC0bGn5UNnwjI= -github.com/nyaruka/goflow v0.64.6 h1:maKHM2yVLeqyFhsU7z4whtFLnfACdWvJ/6V+rPeY8mg= -github.com/nyaruka/goflow v0.64.6/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= +github.com/nyaruka/goflow v0.64.7 h1:5CK/cfsCh04Tpdkj8prgvEZJCQfkwlx8ZvAMRUhMBj8= +github.com/nyaruka/goflow v0.64.7/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= github.com/nyaruka/librato v1.0.0 h1:Vznj9WCeC1yZXbBYyYp40KnbmXLbEkjKmHesV/v2SR0= github.com/nyaruka/librato v1.0.0/go.mod h1:pkRNLFhFurOz0QqBz6/DuTFhHHxAubWxs4Jx+J7yUgg= github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d h1:hyp9u36KIwbTCo2JAJ+TuJcJBc+UZzEig7RI/S5Dvkc= From 3f6d56885ce6789bb9477a3a0347248d10dbb925 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 16 Jan 2020 13:24:22 -0500 Subject: [PATCH 13/24] Update CHANGELOG.md for v5.3.18 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6f4a5de1..2f1cf5527 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +v5.3.18 +---------- + * Update to goflow v0.64.7 + v5.3.17 ---------- * Include evaluation context with simulation requests From df49e3b46592d7011a293e4619a3d580c576c13e Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Fri, 17 Jan 2020 10:36:17 -0500 Subject: [PATCH 14/24] Update to goflow v0.64.8 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 64d3b9f73..b8cd6a3f8 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/mattn/go-sqlite3 v1.10.0 // indirect github.com/nyaruka/ezconf v0.2.1 github.com/nyaruka/gocommon v1.1.1 - github.com/nyaruka/goflow v0.64.7 + github.com/nyaruka/goflow v0.64.8 github.com/nyaruka/librato v1.0.0 github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d github.com/nyaruka/null v1.2.0 diff --git a/go.sum b/go.sum index abf3be0ff..6e7c860be 100644 --- a/go.sum +++ b/go.sum @@ -70,8 +70,8 @@ github.com/nyaruka/ezconf v0.2.1 h1:TDXWoqjqYya1uhou1mAJZg7rgFYL98EB0Tb3+BWtUh0= github.com/nyaruka/ezconf v0.2.1/go.mod h1:ey182kYkw2MIi4XiWe1FR/mzI33WCmTWuceDYYxgnQw= github.com/nyaruka/gocommon v1.1.1 h1:RnQ+kMzN1lA+W0NpkBDd0mGU3UqadJygR3SMpITMYTQ= github.com/nyaruka/gocommon v1.1.1/go.mod h1:QbdU2J9WBsqBmeZRuwndf2f6O7rD7mkC0bGn5UNnwjI= -github.com/nyaruka/goflow v0.64.7 h1:5CK/cfsCh04Tpdkj8prgvEZJCQfkwlx8ZvAMRUhMBj8= -github.com/nyaruka/goflow v0.64.7/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= +github.com/nyaruka/goflow v0.64.8 h1:DDzpBxcn40bxKxIFPFqOqxWVsdPJm5VT7fCa2L//DCI= +github.com/nyaruka/goflow v0.64.8/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= github.com/nyaruka/librato v1.0.0 h1:Vznj9WCeC1yZXbBYyYp40KnbmXLbEkjKmHesV/v2SR0= github.com/nyaruka/librato v1.0.0/go.mod h1:pkRNLFhFurOz0QqBz6/DuTFhHHxAubWxs4Jx+J7yUgg= github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d h1:hyp9u36KIwbTCo2JAJ+TuJcJBc+UZzEig7RI/S5Dvkc= From 82a363c64f3dc9447d63f638b6159c994b36e71a Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Fri, 17 Jan 2020 10:38:03 -0500 Subject: [PATCH 15/24] Update CHANGELOG.md for v5.3.19 --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f1cf5527..cf5a3b2e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +v5.3.19 +---------- + * Update to goflow v0.64.8 + v5.3.18 ---------- * Update to goflow v0.64.7 From 8e8b4ba30bd5a8e874b12e83a216c437921a5eef Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Sat, 18 Jan 2020 18:43:46 -0800 Subject: [PATCH 16/24] update tests --- search/search.go | 8 ++++---- search/testdata/elastic_test.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/search/search.go b/search/search.go index 395df2b9a..8691d940f 100644 --- a/search/search.go +++ b/search/search.go @@ -17,12 +17,12 @@ import ( func ToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, query string) (string, elastic.Query, error) { node, err := contactql.ParseQuery(query, env.RedactionPolicy(), resolver) if err != nil { - return "", nil, NewError("error parsing query: %s", err.Error()) + return "", nil, NewError(err.Error()) } eq, err := nodeToElasticQuery(env, resolver, node.Root()) if err != nil { - return "", nil, errors.Wrapf(err, "error parsing query: %s", query) + return "", nil, NewError(err.Error()) } return node.String(), eq, nil @@ -159,7 +159,7 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol } else if fieldType == assets.FieldTypeDatetime { value, err := envs.DateTimeFromString(env, c.Value(), false) if err != nil { - return nil, NewError("unable to parse datetime: %s", c.Value()) + return nil, NewError("string '%s' couldn't be parsed as a date", c.Value()) } start, end := dates.DayToUTCRange(value, value.Location()) @@ -247,7 +247,7 @@ func conditionToElasticQuery(env envs.Environment, resolver contactql.FieldResol } else if key == contactql.AttributeCreatedOn { value, err := envs.DateTimeFromString(env, c.Value(), false) if err != nil { - return nil, NewError("unable to parse datetime: %s", c.Value()) + return nil, NewError("string '%s' couldn't be parsed as a date", c.Value()) } start, end := dates.DayToUTCRange(value, value.Location()) diff --git a/search/testdata/elastic_test.json b/search/testdata/elastic_test.json index 1dd14326f..c3e8a7d3a 100644 --- a/search/testdata/elastic_test.json +++ b/search/testdata/elastic_test.json @@ -1226,7 +1226,7 @@ { "label": "invalid created_on operand", "search": "created_on Date: Mon, 20 Jan 2020 12:15:05 -0800 Subject: [PATCH 17/24] prevent nil wrapping --- search/search_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/search/search_test.go b/search/search_test.go index ea6afa3d9..dc3eff613 100644 --- a/search/search_test.go +++ b/search/search_test.go @@ -55,7 +55,11 @@ func TestElasticSort(t *testing.T) { } resolver := func(key string) assets.Field { - return registry[key] + field, found := registry[key] + if !found { + return nil + } + return field } for _, tc := range tcs { @@ -100,7 +104,11 @@ func TestElasticQuery(t *testing.T) { ny, _ := time.LoadLocation("America/New_York") resolver := func(key string) assets.Field { - return registry[key] + field, found := registry[key] + if !found { + return nil + } + return field } for _, tc := range tcs { From 6d02f1e8a8f3697039084ef3b4f5fc6b166b4372 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Tue, 21 Jan 2020 12:32:31 -0500 Subject: [PATCH 18/24] Update to latest goflow v0.64.9 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 829a139aa..9f9ed7253 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( github.com/mattn/go-sqlite3 v1.10.0 // indirect github.com/nyaruka/ezconf v0.2.1 github.com/nyaruka/gocommon v1.1.1 - github.com/nyaruka/goflow v0.64.8 + github.com/nyaruka/goflow v0.64.9 github.com/nyaruka/librato v1.0.0 github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d github.com/nyaruka/null v1.2.0 diff --git a/go.sum b/go.sum index 12c28d693..7ef12bf94 100644 --- a/go.sum +++ b/go.sum @@ -75,8 +75,8 @@ github.com/nyaruka/ezconf v0.2.1 h1:TDXWoqjqYya1uhou1mAJZg7rgFYL98EB0Tb3+BWtUh0= github.com/nyaruka/ezconf v0.2.1/go.mod h1:ey182kYkw2MIi4XiWe1FR/mzI33WCmTWuceDYYxgnQw= github.com/nyaruka/gocommon v1.1.1 h1:RnQ+kMzN1lA+W0NpkBDd0mGU3UqadJygR3SMpITMYTQ= github.com/nyaruka/gocommon v1.1.1/go.mod h1:QbdU2J9WBsqBmeZRuwndf2f6O7rD7mkC0bGn5UNnwjI= -github.com/nyaruka/goflow v0.64.8 h1:DDzpBxcn40bxKxIFPFqOqxWVsdPJm5VT7fCa2L//DCI= -github.com/nyaruka/goflow v0.64.8/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= +github.com/nyaruka/goflow v0.64.9 h1:w8XgSrUP2ujYrYIFinyUCZKHGkzL6IyuTYiKfi31gvE= +github.com/nyaruka/goflow v0.64.9/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= github.com/nyaruka/librato v1.0.0 h1:Vznj9WCeC1yZXbBYyYp40KnbmXLbEkjKmHesV/v2SR0= github.com/nyaruka/librato v1.0.0/go.mod h1:pkRNLFhFurOz0QqBz6/DuTFhHHxAubWxs4Jx+J7yUgg= github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d h1:hyp9u36KIwbTCo2JAJ+TuJcJBc+UZzEig7RI/S5Dvkc= From 72633e17e82c2c087b5e12107845c0e1ba8f6256 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Tue, 21 Jan 2020 12:33:29 -0500 Subject: [PATCH 19/24] Update CHANGELOG.md for v5.3.20 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf5a3b2e0..e77b5cbe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +v5.3.20 +---------- + * Update to latest goflow v0.64.9 + * Add contact search web endpoint + v5.3.19 ---------- * Update to goflow v0.64.8 From a263dd894648783438b32ee14c41060e5aa1e7bd Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Tue, 21 Jan 2020 17:38:42 -0800 Subject: [PATCH 20/24] refactor and tests in order to allow returning dependent fields for query --- models/contacts.go | 37 +++++++++++------- search/search.go | 50 +++++++++++++++++++++---- search/search_test.go | 75 ++++++++++++++++++++++++------------- web/contact/contact.go | 4 +- web/contact/contact_test.go | 74 ++++++++++++++++++++++-------------- 5 files changed, 164 insertions(+), 76 deletions(-) diff --git a/models/contacts.go b/models/contacts.go index 824837495..316a79bcf 100644 --- a/models/contacts.go +++ b/models/contacts.go @@ -160,12 +160,12 @@ func buildFieldResolver(org *OrgAssets) contactql.FieldResolverFunc { } } -// ParseQuery parses the passed in query for the passed in org, returning the parsed query and the resulting elastic query -func ParseQuery(org *OrgAssets, resolver contactql.FieldResolverFunc, query string) (string, elastic.Query, error) { +// BuildElasticQuery turns the passed in contact ql query into an elastic query +func BuildElasticQuery(org *OrgAssets, resolver contactql.FieldResolverFunc, query *contactql.ContactQuery) (elastic.Query, error) { // turn into elastic query - parsed, eq, err := search.ToElasticQuery(org.Env(), resolver, query) + eq, err := search.ToElasticQuery(org.Env(), resolver, query) if err != nil { - return "", nil, errors.Wrapf(err, "error converting contactql to elastic query: %s", query) + return nil, errors.Wrapf(err, "error converting contactql to elastic query: %s", query) } // additionally filter by org and active contacts @@ -175,26 +175,32 @@ func ParseQuery(org *OrgAssets, resolver contactql.FieldResolverFunc, query stri elastic.NewTermQuery("is_active", true), ) - return parsed, eq, nil + return eq, nil } // ContactIDsForQueryPage returns the ids of the contacts for the passed in query page -func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *OrgAssets, group assets.GroupUUID, query string, sort string, offset int, pageSize int) (string, []ContactID, int64, error) { +func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *OrgAssets, group assets.GroupUUID, query string, sort string, offset int, pageSize int) (*contactql.ContactQuery, []ContactID, int64, error) { start := time.Now() if client == nil { - return "", nil, 0, errors.Errorf("no elastic client available, check your configuration") + return nil, nil, 0, errors.Errorf("no elastic client available, check your configuration") } resolver := buildFieldResolver(org) - parsed, eq, err := ParseQuery(org, resolver, query) + + parsed, err := search.ParseQuery(org.Env(), resolver, query) + if err != nil { + return nil, nil, 0, errors.Wrapf(err, "error parsind query: %s", query) + } + + eq, err := BuildElasticQuery(org, resolver, parsed) if err != nil { - return "", nil, 0, errors.Wrapf(err, "error parsing query: %s", query) + return nil, nil, 0, errors.Wrapf(err, "error parsing query: %s", query) } fieldSort, err := search.ToElasticFieldSort(resolver, sort) if err != nil { - return "", nil, 0, errors.Wrapf(err, "error parsing sort") + return nil, nil, 0, errors.Wrapf(err, "error parsing sort") } // filter by our base group @@ -208,14 +214,14 @@ func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *Or results, err := s.Do(ctx) if err != nil { - return "", nil, 0, errors.Wrapf(err, "error performing query") + return nil, nil, 0, errors.Wrapf(err, "error performing query") } ids := make([]ContactID, 0, pageSize) for _, hit := range results.Hits.Hits { id, err := strconv.Atoi(hit.Id) if err != nil { - return "", nil, 0, errors.Wrapf(err, "unexpected non-integer contact id: %s for search: %s", hit.Id, query) + return nil, nil, 0, errors.Wrapf(err, "unexpected non-integer contact id: %s for search: %s", hit.Id, query) } ids = append(ids, ContactID(id)) } @@ -243,7 +249,12 @@ func ContactIDsForQuery(ctx context.Context, client *elastic.Client, org *OrgAss // turn into elastic query resolver := buildFieldResolver(org) - _, eq, err := ParseQuery(org, resolver, query) + parsed, err := search.ParseQuery(org.Env(), resolver, query) + if err != nil { + return nil, errors.Wrapf(err, "error parsing query: %s", query) + } + + eq, err := BuildElasticQuery(org, resolver, parsed) if err != nil { return nil, errors.Wrapf(err, "error converting contactql to elastic query: %s", query) } diff --git a/search/search.go b/search/search.go index 8691d940f..c8822f17e 100644 --- a/search/search.go +++ b/search/search.go @@ -2,6 +2,7 @@ package search import ( "fmt" + "sort" "strings" "github.com/nyaruka/goflow/assets" @@ -13,19 +14,54 @@ import ( "github.com/shopspring/decimal" ) -// ToElasticQuery converts a contactql query string to an Elastic query returning the normalized view as well as the elastic query -func ToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, query string) (string, elastic.Query, error) { - node, err := contactql.ParseQuery(query, env.RedactionPolicy(), resolver) +// ParseQuery parses the passed in query returning the result +func ParseQuery(env envs.Environment, resolver contactql.FieldResolverFunc, query string) (*contactql.ContactQuery, error) { + parsed, err := contactql.ParseQuery(query, env.RedactionPolicy(), resolver) if err != nil { - return "", nil, NewError(err.Error()) + return nil, NewError(err.Error()) } + return parsed, nil +} - eq, err := nodeToElasticQuery(env, resolver, node.Root()) +// ToElasticQuery converts a contactql query to an Elastic query returning the normalized view as well as the elastic query +func ToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, query *contactql.ContactQuery) (elastic.Query, error) { + eq, err := nodeToElasticQuery(env, resolver, query.Root()) if err != nil { - return "", nil, NewError(err.Error()) + return nil, NewError(err.Error()) + } + + return eq, nil +} + +// DependentFields returns all the dependent fields for the passed in query. This includes attributes such as "id" and "name" +func DependentFields(query *contactql.ContactQuery) []string { + seen := make(map[string]bool) + var appendFields func(node contactql.QueryNode, seen map[string]bool) + appendFields = func(node contactql.QueryNode, seen map[string]bool) { + switch n := node.(type) { + case *contactql.BoolCombination: + for _, c := range n.Children() { + appendFields(c, seen) + } + + case *contactql.Condition: + seen[n.PropertyKey()] = true + + default: + panic(fmt.Sprintf("unknown type in contactql query: %v", n)) + } } - return node.String(), eq, nil + appendFields(query.Root(), seen) + fields := make([]string, 0, len(seen)) + for k := range seen { + fields = append(fields, k) + } + + // order to make deterministic + sort.Strings(fields) + + return fields } // ToElasticFieldSort returns the FieldSort for the passed in field diff --git a/search/search_test.go b/search/search_test.go index dc3eff613..64b36d999 100644 --- a/search/search_test.go +++ b/search/search_test.go @@ -9,7 +9,9 @@ import ( "time" "github.com/nyaruka/goflow/assets" + "github.com/nyaruka/goflow/contactql" "github.com/nyaruka/goflow/envs" + "github.com/olivere/elastic" "github.com/stretchr/testify/assert" ) @@ -24,7 +26,7 @@ func (f *MockField) Name() string { return f.fieldKey } func (f *MockField) Type() assets.FieldType { return f.fieldType } func (f *MockField) UUID() assets.FieldUUID { return f.fieldUUID } -func TestElasticSort(t *testing.T) { +func buildResolver() contactql.FieldResolverFunc { registry := map[string]assets.Field{ "age": &MockField{"age", assets.FieldTypeNumber, "6b6a43fa-a26d-4017-bede-328bcdd5c93b"}, "color": &MockField{"color", assets.FieldTypeText, "ecc7b13b-c698-4f46-8a90-24a8fab6fe34"}, @@ -34,6 +36,20 @@ func TestElasticSort(t *testing.T) { "ward": &MockField{"ward", assets.FieldTypeWard, "fde8f740-c337-421b-8abb-83b954897c80"}, } + resolver := func(key string) assets.Field { + field, found := registry[key] + if !found { + return nil + } + return field + } + + return resolver +} + +func TestElasticSort(t *testing.T) { + resolver := buildResolver() + tcs := []struct { Label string Sort string @@ -54,14 +70,6 @@ func TestElasticSort(t *testing.T) { {"unknown field", "foo", "", fmt.Errorf("unable to find field with name: foo")}, } - resolver := func(key string) assets.Field { - field, found := registry[key] - if !found { - return nil - } - return field - } - for _, tc := range tcs { sort, err := ToElasticFieldSort(resolver, tc.Sort) @@ -76,16 +84,33 @@ func TestElasticSort(t *testing.T) { } } -func TestElasticQuery(t *testing.T) { - registry := map[string]assets.Field{ - "age": &MockField{"age", assets.FieldTypeNumber, "6b6a43fa-a26d-4017-bede-328bcdd5c93b"}, - "color": &MockField{"color", assets.FieldTypeText, "ecc7b13b-c698-4f46-8a90-24a8fab6fe34"}, - "dob": &MockField{"dob", assets.FieldTypeDatetime, "cbd3fc0e-9b74-4207-a8c7-248082bb4572"}, - "state": &MockField{"state", assets.FieldTypeState, "67663ad1-3abc-42dd-a162-09df2dea66ec"}, - "district": &MockField{"district", assets.FieldTypeDistrict, "54c72635-d747-4e45-883c-099d57dd998e"}, - "ward": &MockField{"ward", assets.FieldTypeWard, "fde8f740-c337-421b-8abb-83b954897c80"}, +func TestQueryTerms(t *testing.T) { + resolver := buildResolver() + + tcs := []struct { + Query string + Fields []string + }{ + {"joe", []string{"name"}}, + {"id = 10", []string{"id"}}, + {"name = joe or age > 10", []string{"age", "name"}}, } + env := envs.NewBuilder().Build() + + for _, tc := range tcs { + parsed, err := ParseQuery(env, resolver, tc.Query) + assert.NoError(t, err) + + fields := DependentFields(parsed) + assert.Equal(t, fields, tc.Fields) + } + +} + +func TestElasticQuery(t *testing.T) { + resolver := buildResolver() + type TestCase struct { Label string `json:"label"` Search string `json:"search"` @@ -93,7 +118,6 @@ func TestElasticQuery(t *testing.T) { Error string `json:"error"` IsAnon bool `json:"is_anon"` } - tcs := make([]TestCase, 0, 20) tcJSON, err := ioutil.ReadFile("testdata/elastic_test.json") assert.NoError(t, err) @@ -103,14 +127,6 @@ func TestElasticQuery(t *testing.T) { ny, _ := time.LoadLocation("America/New_York") - resolver := func(key string) assets.Field { - field, found := registry[key] - if !found { - return nil - } - return field - } - for _, tc := range tcs { redactionPolicy := envs.RedactionPolicyNone if tc.IsAnon { @@ -118,7 +134,12 @@ func TestElasticQuery(t *testing.T) { } env := envs.NewBuilder().WithTimezone(ny).WithRedactionPolicy(redactionPolicy).Build() - _, query, err := ToElasticQuery(env, resolver, tc.Search) + qlQuery, err := ParseQuery(env, resolver, tc.Search) + + var query elastic.Query + if err == nil { + query, err = ToElasticQuery(env, resolver, qlQuery) + } if tc.Error != "" { assert.Error(t, err, "%s: error not received converting to elastic: %s", tc.Label, tc.Search) diff --git a/web/contact/contact.go b/web/contact/contact.go index 69ddce69d..ed95e7cb9 100644 --- a/web/contact/contact.go +++ b/web/contact/contact.go @@ -45,6 +45,7 @@ type searchRequest struct { type searchResponse struct { Query string `json:"query"` ContactIDs []models.ContactID `json:"contact_ids"` + Fields []string `json:"fields"` Total int64 `json:"total"` Offset int `json:"offset"` Sort string `json:"sort"` @@ -82,8 +83,9 @@ func handleSearch(ctx context.Context, s *web.Server, r *http.Request) (interfac // build our response response := &searchResponse{ - Query: parsed, + Query: parsed.String(), ContactIDs: hits, + Fields: search.DependentFields(parsed), Total: total, Offset: request.Offset, Sort: request.Sort, diff --git a/web/contact/contact_test.go b/web/contact/contact_test.go index 687d425c2..0ee81b607 100644 --- a/web/contact/contact_test.go +++ b/web/contact/contact_test.go @@ -46,6 +46,34 @@ func TestServer(t *testing.T) { defer server.Stop() + singleESResponse := fmt.Sprintf(`{ + "_scroll_id": "DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAbgc0WS1hqbHlfb01SM2lLTWJRMnVOSVZDdw==", + "took": 2, + "timed_out": false, + "_shards": { + "total": 1, + "successful": 1, + "skipped": 0, + "failed": 0 + }, + "hits": { + "total": 1, + "max_score": null, + "hits": [ + { + "_index": "contacts", + "_type": "_doc", + "_id": "%d", + "_score": null, + "_routing": "1", + "sort": [ + 15124352 + ] + } + ] + } + }`, models.CathyID) + tcs := []struct { URL string Method string @@ -53,42 +81,30 @@ func TestServer(t *testing.T) { Status int Response string Hits []models.ContactID + Query string + Fields []string ESResponse string }{ - {"/mr/contact/search", "GET", "", 405, "illegal", nil, ""}, + {"/mr/contact/search", "GET", "", 405, "illegal", nil, "", nil, ""}, { "/mr/contact/search", "POST", fmt.Sprintf(`{"org_id": 1, "query": "Cathy", "group_uuid": "%s"}`, models.AllContactsGroupUUID), 200, `name~Cathy`, []models.ContactID{models.CathyID}, - fmt.Sprintf(`{ - "_scroll_id": "DXF1ZXJ5QW5kRmV0Y2gBAAAAAAAbgc0WS1hqbHlfb01SM2lLTWJRMnVOSVZDdw==", - "took": 2, - "timed_out": false, - "_shards": { - "total": 1, - "successful": 1, - "skipped": 0, - "failed": 0 - }, - "hits": { - "total": 1, - "max_score": null, - "hits": [ - { - "_index": "contacts", - "_type": "_doc", - "_id": "%d", - "_score": null, - "_routing": "1", - "sort": [ - 15124352 - ] - } - ] - } - }`, models.CathyID), + `name~Cathy`, + []string{"name"}, + singleESResponse, + }, + { + "/mr/contact/search", "POST", + fmt.Sprintf(`{"org_id": 1, "query": "age = 10 and gender = M", "group_uuid": "%s"}`, models.AllContactsGroupUUID), + 200, + `AND(age=10, gender=M)`, + []models.ContactID{models.CathyID}, + `AND(age=10, gender=M)`, + []string{"age", "gender"}, + singleESResponse, }, } @@ -118,6 +134,8 @@ func TestServer(t *testing.T) { err = json.Unmarshal(content, r) assert.NoError(t, err) assert.Equal(t, tc.Hits, r.ContactIDs) + assert.Equal(t, tc.Query, r.Query) + assert.Equal(t, tc.Fields, r.Fields) } } } From 6447c3c4c8c856f43a9b8c7d01e01603e527ae96 Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Wed, 22 Jan 2020 12:31:09 -0800 Subject: [PATCH 21/24] latest goflow, review tweaks --- go.mod | 2 +- go.sum | 2 ++ models/contacts.go | 2 +- search/search.go | 4 ++-- search/search_test.go | 6 +++--- web/contact/contact.go | 2 +- web/contact/contact_test.go | 33 ++++++++++++++++++++++++--------- web/server.go | 18 +++++++++++------- 8 files changed, 45 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index 829a139aa..9cf65a668 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( github.com/mattn/go-sqlite3 v1.10.0 // indirect github.com/nyaruka/ezconf v0.2.1 github.com/nyaruka/gocommon v1.1.1 - github.com/nyaruka/goflow v0.64.8 + github.com/nyaruka/goflow v0.64.10 github.com/nyaruka/librato v1.0.0 github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d github.com/nyaruka/null v1.2.0 diff --git a/go.sum b/go.sum index 12c28d693..9a22bfe55 100644 --- a/go.sum +++ b/go.sum @@ -77,6 +77,8 @@ github.com/nyaruka/gocommon v1.1.1 h1:RnQ+kMzN1lA+W0NpkBDd0mGU3UqadJygR3SMpITMYT github.com/nyaruka/gocommon v1.1.1/go.mod h1:QbdU2J9WBsqBmeZRuwndf2f6O7rD7mkC0bGn5UNnwjI= github.com/nyaruka/goflow v0.64.8 h1:DDzpBxcn40bxKxIFPFqOqxWVsdPJm5VT7fCa2L//DCI= github.com/nyaruka/goflow v0.64.8/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= +github.com/nyaruka/goflow v0.64.10 h1:vBfE8J1Uuw/jo9hZd5C3G1dLIUc/Zz6ioQODRB8oi0I= +github.com/nyaruka/goflow v0.64.10/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= github.com/nyaruka/librato v1.0.0 h1:Vznj9WCeC1yZXbBYyYp40KnbmXLbEkjKmHesV/v2SR0= github.com/nyaruka/librato v1.0.0/go.mod h1:pkRNLFhFurOz0QqBz6/DuTFhHHxAubWxs4Jx+J7yUgg= github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d h1:hyp9u36KIwbTCo2JAJ+TuJcJBc+UZzEig7RI/S5Dvkc= diff --git a/models/contacts.go b/models/contacts.go index 316a79bcf..28ea21b98 100644 --- a/models/contacts.go +++ b/models/contacts.go @@ -190,7 +190,7 @@ func ContactIDsForQueryPage(ctx context.Context, client *elastic.Client, org *Or parsed, err := search.ParseQuery(org.Env(), resolver, query) if err != nil { - return nil, nil, 0, errors.Wrapf(err, "error parsind query: %s", query) + return nil, nil, 0, errors.Wrapf(err, "error parsing query: %s", query) } eq, err := BuildElasticQuery(org, resolver, parsed) diff --git a/search/search.go b/search/search.go index c8822f17e..513937575 100644 --- a/search/search.go +++ b/search/search.go @@ -33,8 +33,8 @@ func ToElasticQuery(env envs.Environment, resolver contactql.FieldResolverFunc, return eq, nil } -// DependentFields returns all the dependent fields for the passed in query. This includes attributes such as "id" and "name" -func DependentFields(query *contactql.ContactQuery) []string { +// FieldDependencies returns all the field this query is dependent on. This includes attributes such as "id" and "name" +func FieldDependencies(query *contactql.ContactQuery) []string { seen := make(map[string]bool) var appendFields func(node contactql.QueryNode, seen map[string]bool) appendFields = func(node contactql.QueryNode, seen map[string]bool) { diff --git a/search/search_test.go b/search/search_test.go index 64b36d999..d65616a8f 100644 --- a/search/search_test.go +++ b/search/search_test.go @@ -60,7 +60,7 @@ func TestElasticSort(t *testing.T) { {"descending created_on", "-created_on", `{"created_on":{"order":"desc"}}`, nil}, {"ascending name", "name", `{"name":{"order":"asc"}}`, nil}, {"descending language", "-language", `{"language":{"order":"desc"}}`, nil}, - {"descending numeric", "-age", `{"fields.number":{"nested":{"filter":{"term":{"fields.field":"6b6a43fa-a26d-4017-bede-328bcdd5c93b"}},"path":"fields"},"order":"desc"}}`, nil}, + {"descending numeric", "-AGE", `{"fields.number":{"nested":{"filter":{"term":{"fields.field":"6b6a43fa-a26d-4017-bede-328bcdd5c93b"}},"path":"fields"},"order":"desc"}}`, nil}, {"ascending text", "color", `{"fields.text":{"nested":{"filter":{"term":{"fields.field":"ecc7b13b-c698-4f46-8a90-24a8fab6fe34"}},"path":"fields"},"order":"asc"}}`, nil}, {"descending date", "-dob", `{"fields.datetime":{"nested":{"filter":{"term":{"fields.field":"cbd3fc0e-9b74-4207-a8c7-248082bb4572"}},"path":"fields"},"order":"desc"}}`, nil}, {"descending state", "-state", `{"fields.state":{"nested":{"filter":{"term":{"fields.field":"67663ad1-3abc-42dd-a162-09df2dea66ec"}},"path":"fields"},"order":"desc"}}`, nil}, @@ -93,7 +93,7 @@ func TestQueryTerms(t *testing.T) { }{ {"joe", []string{"name"}}, {"id = 10", []string{"id"}}, - {"name = joe or age > 10", []string{"age", "name"}}, + {"name = joe or AGE > 10", []string{"age", "name"}}, } env := envs.NewBuilder().Build() @@ -102,7 +102,7 @@ func TestQueryTerms(t *testing.T) { parsed, err := ParseQuery(env, resolver, tc.Query) assert.NoError(t, err) - fields := DependentFields(parsed) + fields := FieldDependencies(parsed) assert.Equal(t, fields, tc.Fields) } diff --git a/web/contact/contact.go b/web/contact/contact.go index ed95e7cb9..45df69df9 100644 --- a/web/contact/contact.go +++ b/web/contact/contact.go @@ -85,7 +85,7 @@ func handleSearch(ctx context.Context, s *web.Server, r *http.Request) (interfac response := &searchResponse{ Query: parsed.String(), ContactIDs: hits, - Fields: search.DependentFields(parsed), + Fields: search.FieldDependencies(parsed), Total: total, Offset: request.Offset, Sort: request.Sort, diff --git a/web/contact/contact_test.go b/web/contact/contact_test.go index 0ee81b607..87f656228 100644 --- a/web/contact/contact_test.go +++ b/web/contact/contact_test.go @@ -7,7 +7,6 @@ import ( "io" "io/ioutil" "net/http" - "strings" "sync" "testing" "time" @@ -79,30 +78,42 @@ func TestServer(t *testing.T) { Method string Body string Status int - Response string + Error string Hits []models.ContactID Query string Fields []string ESResponse string }{ - {"/mr/contact/search", "GET", "", 405, "illegal", nil, "", nil, ""}, + {"/mr/contact/search", "GET", "", 405, "illegal method: GET", nil, "", nil, ""}, + { + "/mr/contact/search", "POST", + fmt.Sprintf(`{"org_id": 1, "query": "birthday = tomorrow", "group_uuid": "%s"}`, models.AllContactsGroupUUID), + 400, "can't resolve 'birthday' to attribute, scheme or field", + nil, "", nil, "", + }, + { + "/mr/contact/search", "POST", + fmt.Sprintf(`{"org_id": 1, "query": "age > tomorrow", "group_uuid": "%s"}`, models.AllContactsGroupUUID), + 400, "can't convert 'tomorrow' to a number", + nil, "", nil, "", + }, { "/mr/contact/search", "POST", fmt.Sprintf(`{"org_id": 1, "query": "Cathy", "group_uuid": "%s"}`, models.AllContactsGroupUUID), 200, - `name~Cathy`, + "", []models.ContactID{models.CathyID}, - `name~Cathy`, + `name ~ "Cathy"`, []string{"name"}, singleESResponse, }, { "/mr/contact/search", "POST", - fmt.Sprintf(`{"org_id": 1, "query": "age = 10 and gender = M", "group_uuid": "%s"}`, models.AllContactsGroupUUID), + fmt.Sprintf(`{"org_id": 1, "query": "AGE = 10 and gender = M", "group_uuid": "%s"}`, models.AllContactsGroupUUID), 200, - `AND(age=10, gender=M)`, + "", []models.ContactID{models.CathyID}, - `AND(age=10, gender=M)`, + `age = 10 AND gender = "M"`, []string{"age", "gender"}, singleESResponse, }, @@ -126,7 +137,6 @@ func TestServer(t *testing.T) { content, err := ioutil.ReadAll(resp.Body) assert.NoError(t, err, "%d: error reading body", i) - assert.True(t, strings.Contains(string(content), tc.Response), "%d: did not find string: %s in body: %s", i, tc.Response, string(content)) // on 200 responses parse them if resp.StatusCode == 200 { @@ -136,6 +146,11 @@ func TestServer(t *testing.T) { assert.Equal(t, tc.Hits, r.ContactIDs) assert.Equal(t, tc.Query, r.Query) assert.Equal(t, tc.Fields, r.Fields) + } else { + r := &web.ErrorResponse{} + err = json.Unmarshal(content, r) + assert.NoError(t, err) + assert.Equal(t, tc.Error, r.Error) } } } diff --git a/web/server.go b/web/server.go index d0bd19467..6530ed5d0 100644 --- a/web/server.go +++ b/web/server.go @@ -182,12 +182,12 @@ func (s *Server) WrapJSONHandler(handler JSONHandler) http.HandlerFunc { // handler errored (a hard error) if err != nil { - value = errorAsResponse(err) + value = NewErrorResponse(err) } else { // handler returned an error to use as a the response asError, isError := value.(error) if isError { - value = errorAsResponse(asError) + value = NewErrorResponse(asError) } } @@ -221,7 +221,7 @@ func (s *Server) WrapHandler(handler Handler) http.HandlerFunc { logrus.WithError(err).WithField("http_request", r).Error("error handling request") w.WriteHeader(http.StatusInternalServerError) - serialized, _ := json.Marshal(errorAsResponse(err)) + serialized, _ := json.Marshal(NewErrorResponse(err)) w.Write(serialized) return } @@ -285,8 +285,12 @@ type Server struct { httpServer *http.Server } -func errorAsResponse(err error) interface{} { - return map[string]string{ - "error": err.Error(), - } +// ErrorResponse is the type for our error responses, it just contains a single error field +type ErrorResponse struct { + Error string `json:"error"` +} + +// NewErrorResponse creates a new error response from the passed in errro +func NewErrorResponse(err error) *ErrorResponse { + return &ErrorResponse{err.Error()} } From 840fb23cbc7aa4357ca667ef2282572111d56034 Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Wed, 22 Jan 2020 13:42:11 -0800 Subject: [PATCH 22/24] latest goflow --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 9cf65a668..939190006 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( github.com/mattn/go-sqlite3 v1.10.0 // indirect github.com/nyaruka/ezconf v0.2.1 github.com/nyaruka/gocommon v1.1.1 - github.com/nyaruka/goflow v0.64.10 + github.com/nyaruka/goflow v0.64.11 github.com/nyaruka/librato v1.0.0 github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d github.com/nyaruka/null v1.2.0 diff --git a/go.sum b/go.sum index 9e2e11b57..11d62786b 100644 --- a/go.sum +++ b/go.sum @@ -77,6 +77,8 @@ github.com/nyaruka/gocommon v1.1.1 h1:RnQ+kMzN1lA+W0NpkBDd0mGU3UqadJygR3SMpITMYT github.com/nyaruka/gocommon v1.1.1/go.mod h1:QbdU2J9WBsqBmeZRuwndf2f6O7rD7mkC0bGn5UNnwjI= github.com/nyaruka/goflow v0.64.10 h1:vBfE8J1Uuw/jo9hZd5C3G1dLIUc/Zz6ioQODRB8oi0I= github.com/nyaruka/goflow v0.64.10/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= +github.com/nyaruka/goflow v0.64.11 h1:ErJnURsTjossFzs1mzKN8vEfQAUrbc7xFCU1om4RliI= +github.com/nyaruka/goflow v0.64.11/go.mod h1:fb6eGAXiTL2hjbzMpXwSIlNx0O4IsR9XBunXBN+8pWM= github.com/nyaruka/librato v1.0.0 h1:Vznj9WCeC1yZXbBYyYp40KnbmXLbEkjKmHesV/v2SR0= github.com/nyaruka/librato v1.0.0/go.mod h1:pkRNLFhFurOz0QqBz6/DuTFhHHxAubWxs4Jx+J7yUgg= github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d h1:hyp9u36KIwbTCo2JAJ+TuJcJBc+UZzEig7RI/S5Dvkc= From 9c400da9478483334685c2fbafc19a89c3b81315 Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Wed, 22 Jan 2020 13:42:56 -0800 Subject: [PATCH 23/24] Update CHANGELOG.md for v5.3.21 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e77b5cbe1..82c811f1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +v5.3.21 +---------- + * Return field dependencies with queries on contact search endpoint + * Latest goflow, larger webhook bodies, trim expressions + v5.3.20 ---------- * Update to latest goflow v0.64.9 From aeba50d0a30142f1b7558809493adb7ad00d58b6 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Thu, 23 Jan 2020 10:47:01 -0500 Subject: [PATCH 24/24] Make default for MaxBodyBytes 1MB --- config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 5212af12d..68cce57e8 100644 --- a/config/config.go +++ b/config/config.go @@ -67,7 +67,7 @@ func NewMailroomConfig() *Config { WebhooksTimeout: 15000, WebhooksMaxRetries: 2, - WebhooksMaxBodyBytes: 10000, + WebhooksMaxBodyBytes: 1024 * 1024, // 1MB WebhooksInitialBackoff: 5000, WebhooksBackoffJitter: 0.5, SMTPServer: "",