From c5bcdd3a2255d3b018c62b245b5753a8d6c81829 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Mon, 28 Oct 2019 02:37:58 -0400 Subject: [PATCH] libs/pubsub: relax tx querying (#4070) Some linting/cleanup missed from the initial events refactor Don't panic; instead, return false, error when matching breaks unexpectedly Strip non-numeric chars from values when attempting to match against query values Have the server log during send upon error * cleanup/lint Query#Conditions and do not panic * cleanup/lint Query#Matches and do not panic * cleanup/lint matchValue and do not panic * rever to panic in Query#Conditions * linting * strip alpha chars when attempting to match * add pending log entries * Update libs/pubsub/query/query.go Co-Authored-By: Anton Kaliaev * build: update variable names * update matchValue to return an error * update Query#Matches to return an error * update TestMatches * log error in send * Fix tests * Fix TestEmptyQueryMatchesAnything * fix linting * Update libs/pubsub/query/query.go Co-Authored-By: Anton Kaliaev * Update libs/pubsub/query/query.go Co-Authored-By: Anton Kaliaev * Update libs/pubsub/query/query.go Co-Authored-By: Anton Kaliaev * Update libs/pubsub/query/query.go Co-Authored-By: Anton Kaliaev * Update libs/pubsub/query/query.go Co-Authored-By: Anton Kaliaev * Update libs/pubsub/pubsub.go Co-Authored-By: Anton Kaliaev * add missing errors pkg import * update Query#Conditions to return an error * update query pkg unit tests * update TxIndex#Search * update pending changelog --- CHANGELOG_PENDING.md | 3 + libs/pubsub/pubsub.go | 20 ++- libs/pubsub/query/empty.go | 4 +- libs/pubsub/query/empty_test.go | 19 ++- libs/pubsub/query/query.go | 290 +++++++++++++++++++++----------- libs/pubsub/query/query_test.go | 75 ++++++--- state/txindex/kv/kv.go | 5 +- 7 files changed, 276 insertions(+), 140 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 689b2ed9b..5a5cb7095 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -15,6 +15,7 @@ program](https://hackerone.com/tendermint). - Apps - Go API + - [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) `Query#(Matches|Conditions)` returns an error. ### FEATURES: @@ -22,6 +23,8 @@ program](https://hackerone.com/tendermint). - [mempool] [\#4057](https://github.com/tendermint/tendermint/issues/4057) Include peer ID when logging rejected txns (@erikgrinaker) - [tools] [\#4023](https://github.com/tendermint/tendermint/issues/4023) Improved `tm-monitor` formatting of start time and avg tx throughput (@erikgrinaker) +- [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) No longer panic in `Query#(Matches|Conditions)` preferring to return an error instead. +- [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) Strip out non-numeric characters when attempting to match numeric values. - [p2p] [\#3991](https://github.com/tendermint/tendermint/issues/3991) Log "has been established or dialed" as debug log instead of Error for connected peers (@whunmr) ### BUG FIXES: diff --git a/libs/pubsub/pubsub.go b/libs/pubsub/pubsub.go index 27e6c4950..bead2aae0 100644 --- a/libs/pubsub/pubsub.go +++ b/libs/pubsub/pubsub.go @@ -36,9 +36,9 @@ package pubsub import ( "context" - "errors" "sync" + "github.com/pkg/errors" cmn "github.com/tendermint/tendermint/libs/common" ) @@ -68,7 +68,7 @@ var ( // allows event types to repeat themselves with the same set of keys and // different values. type Query interface { - Matches(events map[string][]string) bool + Matches(events map[string][]string) (bool, error) String() string } @@ -334,7 +334,9 @@ loop: case sub: state.add(cmd.clientID, cmd.query, cmd.subscription) case pub: - state.send(cmd.msg, cmd.events) + if err := state.send(cmd.msg, cmd.events); err != nil { + s.Logger.Error("Error querying for events", "err", err) + } } } } @@ -401,10 +403,16 @@ func (state *state) removeAll(reason error) { } } -func (state *state) send(msg interface{}, events map[string][]string) { +func (state *state) send(msg interface{}, events map[string][]string) error { for qStr, clientSubscriptions := range state.subscriptions { q := state.queries[qStr].q - if q.Matches(events) { + + match, err := q.Matches(events) + if err != nil { + return errors.Wrapf(err, "failed to match against query %s", q.String()) + } + + if match { for clientID, subscription := range clientSubscriptions { if cap(subscription.out) == 0 { // block on unbuffered channel @@ -420,4 +428,6 @@ func (state *state) send(msg interface{}, events map[string][]string) { } } } + + return nil } diff --git a/libs/pubsub/query/empty.go b/libs/pubsub/query/empty.go index 2d7642adc..2c0d00095 100644 --- a/libs/pubsub/query/empty.go +++ b/libs/pubsub/query/empty.go @@ -5,8 +5,8 @@ type Empty struct { } // Matches always returns true. -func (Empty) Matches(tags map[string][]string) bool { - return true +func (Empty) Matches(tags map[string][]string) (bool, error) { + return true, nil } func (Empty) String() string { diff --git a/libs/pubsub/query/empty_test.go b/libs/pubsub/query/empty_test.go index 3fcd2d728..1b6ef2828 100644 --- a/libs/pubsub/query/empty_test.go +++ b/libs/pubsub/query/empty_test.go @@ -10,8 +10,19 @@ import ( func TestEmptyQueryMatchesAnything(t *testing.T) { q := query.Empty{} - assert.True(t, q.Matches(map[string][]string{})) - assert.True(t, q.Matches(map[string][]string{"Asher": {"Roth"}})) - assert.True(t, q.Matches(map[string][]string{"Route": {"66"}})) - assert.True(t, q.Matches(map[string][]string{"Route": {"66"}, "Billy": {"Blue"}})) + + testCases := []struct { + query map[string][]string + }{ + {map[string][]string{}}, + {map[string][]string{"Asher": {"Roth"}}}, + {map[string][]string{"Route": {"66"}}}, + {map[string][]string{"Route": {"66"}, "Billy": {"Blue"}}}, + } + + for _, tc := range testCases { + match, err := q.Matches(tc.query) + assert.Nil(t, err) + assert.True(t, match) + } } diff --git a/libs/pubsub/query/query.go b/libs/pubsub/query/query.go index 039038a5f..6d423dfeb 100644 --- a/libs/pubsub/query/query.go +++ b/libs/pubsub/query/query.go @@ -11,9 +11,16 @@ package query import ( "fmt" "reflect" + "regexp" "strconv" "strings" "time" + + "github.com/pkg/errors" +) + +var ( + numRegex = regexp.MustCompile(`([0-9\.]+)`) ) // Query holds the query string and the query parser. @@ -82,209 +89,283 @@ const ( TimeLayout = time.RFC3339 ) -// Conditions returns a list of conditions. -func (q *Query) Conditions() []Condition { - conditions := make([]Condition, 0) +// Conditions returns a list of conditions. It returns an error if there is any +// error with the provided grammar in the Query. +func (q *Query) Conditions() ([]Condition, error) { + var ( + eventAttr string + op Operator + ) + conditions := make([]Condition, 0) buffer, begin, end := q.parser.Buffer, 0, 0 - var tag string - var op Operator - - // tokens must be in the following order: tag ("tx.gas") -> operator ("=") -> operand ("7") + // tokens must be in the following order: event attribute ("tx.gas") -> operator ("=") -> operand ("7") for _, token := range q.parser.Tokens() { switch token.pegRule { - case rulePegText: begin, end = int(token.begin), int(token.end) + case ruletag: - tag = buffer[begin:end] + eventAttr = buffer[begin:end] + case rulele: op = OpLessEqual + case rulege: op = OpGreaterEqual + case rulel: op = OpLess + case ruleg: op = OpGreater + case ruleequal: op = OpEqual + case rulecontains: op = OpContains + case rulevalue: // strip single quotes from value (i.e. "'NewBlock'" -> "NewBlock") valueWithoutSingleQuotes := buffer[begin+1 : end-1] - conditions = append(conditions, Condition{tag, op, valueWithoutSingleQuotes}) + conditions = append(conditions, Condition{eventAttr, op, valueWithoutSingleQuotes}) + case rulenumber: number := buffer[begin:end] if strings.ContainsAny(number, ".") { // if it looks like a floating-point number value, err := strconv.ParseFloat(number, 64) if err != nil { - panic(fmt.Sprintf("got %v while trying to parse %s as float64 (should never happen if the grammar is correct)", - err, - number)) + err = fmt.Errorf( + "got %v while trying to parse %s as float64 (should never happen if the grammar is correct)", + err, number, + ) + return nil, err } - conditions = append(conditions, Condition{tag, op, value}) + + conditions = append(conditions, Condition{eventAttr, op, value}) } else { value, err := strconv.ParseInt(number, 10, 64) if err != nil { - panic(fmt.Sprintf("got %v while trying to parse %s as int64 (should never happen if the grammar is correct)", - err, - number)) + err = fmt.Errorf( + "got %v while trying to parse %s as int64 (should never happen if the grammar is correct)", + err, number, + ) + return nil, err } - conditions = append(conditions, Condition{tag, op, value}) + + conditions = append(conditions, Condition{eventAttr, op, value}) } + case ruletime: value, err := time.Parse(TimeLayout, buffer[begin:end]) if err != nil { - panic(fmt.Sprintf( + err = fmt.Errorf( "got %v while trying to parse %s as time.Time / RFC3339 (should never happen if the grammar is correct)", - err, - buffer[begin:end])) + err, buffer[begin:end], + ) + return nil, err } - conditions = append(conditions, Condition{tag, op, value}) + + conditions = append(conditions, Condition{eventAttr, op, value}) + case ruledate: value, err := time.Parse("2006-01-02", buffer[begin:end]) if err != nil { - panic(fmt.Sprintf( + err = fmt.Errorf( "got %v while trying to parse %s as time.Time / '2006-01-02' (should never happen if the grammar is correct)", - err, - buffer[begin:end])) + err, buffer[begin:end], + ) + return nil, err } - conditions = append(conditions, Condition{tag, op, value}) + + conditions = append(conditions, Condition{eventAttr, op, value}) } } - return conditions + return conditions, nil } // Matches returns true if the query matches against any event in the given set // of events, false otherwise. For each event, a match exists if the query is -// matched against *any* value in a slice of values. +// matched against *any* value in a slice of values. An error is returned if +// any attempted event match returns an error. // // For example, query "name=John" matches events = {"name": ["John", "Eric"]}. // More examples could be found in parser_test.go and query_test.go. -func (q *Query) Matches(events map[string][]string) bool { +func (q *Query) Matches(events map[string][]string) (bool, error) { if len(events) == 0 { - return false + return false, nil } - buffer, begin, end := q.parser.Buffer, 0, 0 + var ( + eventAttr string + op Operator + ) - var tag string - var op Operator + buffer, begin, end := q.parser.Buffer, 0, 0 // tokens must be in the following order: - // tag ("tx.gas") -> operator ("=") -> operand ("7") + // event attribute ("tx.gas") -> operator ("=") -> operand ("7") for _, token := range q.parser.Tokens() { switch token.pegRule { - case rulePegText: begin, end = int(token.begin), int(token.end) + case ruletag: - tag = buffer[begin:end] + eventAttr = buffer[begin:end] + case rulele: op = OpLessEqual + case rulege: op = OpGreaterEqual + case rulel: op = OpLess + case ruleg: op = OpGreater + case ruleequal: op = OpEqual + case rulecontains: op = OpContains + case rulevalue: // strip single quotes from value (i.e. "'NewBlock'" -> "NewBlock") valueWithoutSingleQuotes := buffer[begin+1 : end-1] - // see if the triplet (tag, operator, operand) matches any tag + // see if the triplet (event attribute, operator, operand) matches any event // "tx.gas", "=", "7", { "tx.gas": 7, "tx.ID": "4AE393495334" } - if !match(tag, op, reflect.ValueOf(valueWithoutSingleQuotes), events) { - return false + match, err := match(eventAttr, op, reflect.ValueOf(valueWithoutSingleQuotes), events) + if err != nil { + return false, err + } + + if !match { + return false, nil } + case rulenumber: number := buffer[begin:end] if strings.ContainsAny(number, ".") { // if it looks like a floating-point number value, err := strconv.ParseFloat(number, 64) if err != nil { - panic(fmt.Sprintf( + err = fmt.Errorf( "got %v while trying to parse %s as float64 (should never happen if the grammar is correct)", - err, - number)) + err, number, + ) + return false, err } - if !match(tag, op, reflect.ValueOf(value), events) { - return false + + match, err := match(eventAttr, op, reflect.ValueOf(value), events) + if err != nil { + return false, err + } + + if !match { + return false, nil } } else { value, err := strconv.ParseInt(number, 10, 64) if err != nil { - panic(fmt.Sprintf( + err = fmt.Errorf( "got %v while trying to parse %s as int64 (should never happen if the grammar is correct)", - err, - number)) + err, number, + ) + return false, err } - if !match(tag, op, reflect.ValueOf(value), events) { - return false + + match, err := match(eventAttr, op, reflect.ValueOf(value), events) + if err != nil { + return false, err + } + + if !match { + return false, nil } } + case ruletime: value, err := time.Parse(TimeLayout, buffer[begin:end]) if err != nil { - panic(fmt.Sprintf( + err = fmt.Errorf( "got %v while trying to parse %s as time.Time / RFC3339 (should never happen if the grammar is correct)", - err, - buffer[begin:end])) + err, buffer[begin:end], + ) + return false, err } - if !match(tag, op, reflect.ValueOf(value), events) { - return false + + match, err := match(eventAttr, op, reflect.ValueOf(value), events) + if err != nil { + return false, err + } + + if !match { + return false, nil } + case ruledate: value, err := time.Parse("2006-01-02", buffer[begin:end]) if err != nil { - panic(fmt.Sprintf( + err = fmt.Errorf( "got %v while trying to parse %s as time.Time / '2006-01-02' (should never happen if the grammar is correct)", - err, - buffer[begin:end])) + err, buffer[begin:end], + ) + return false, err } - if !match(tag, op, reflect.ValueOf(value), events) { - return false + + match, err := match(eventAttr, op, reflect.ValueOf(value), events) + if err != nil { + return false, err + } + + if !match { + return false, nil } } } - return true + return true, nil } -// match returns true if the given triplet (tag, operator, operand) matches any -// value in an event for that key. +// match returns true if the given triplet (attribute, operator, operand) matches +// any value in an event for that attribute. If any match fails with an error, +// that error is returned. // // First, it looks up the key in the events and if it finds one, tries to compare // all the values from it to the operand using the operator. // // "tx.gas", "=", "7", {"tx": [{"gas": 7, "ID": "4AE393495334"}]} -func match(tag string, op Operator, operand reflect.Value, events map[string][]string) bool { +func match(attr string, op Operator, operand reflect.Value, events map[string][]string) (bool, error) { // look up the tag from the query in tags - values, ok := events[tag] + values, ok := events[attr] if !ok { - return false + return false, nil } for _, value := range values { // return true if any value in the set of the event's values matches - if matchValue(value, op, operand) { - return true + match, err := matchValue(value, op, operand) + if err != nil { + return false, err + } + + if match { + return true, nil } } - return false + return false, nil } -// matchValue will attempt to match a string value against an operation an -// operand. A boolean is returned representing the match result. It will panic -// if an error occurs or if the operand is invalid. -func matchValue(value string, op Operator, operand reflect.Value) bool { +// matchValue will attempt to match a string value against an operator an +// operand. A boolean is returned representing the match result. It will return +// an error if the value cannot be parsed and matched against the operand type. +func matchValue(value string, op Operator, operand reflect.Value) (bool, error) { switch operand.Kind() { case reflect.Struct: // time operandAsTime := operand.Interface().(time.Time) @@ -301,87 +382,94 @@ func matchValue(value string, op Operator, operand reflect.Value) bool { v, err = time.Parse(DateLayout, value) } if err != nil { - panic(fmt.Sprintf("failed to convert value %v from tag to time.Time: %v", value, err)) + return false, errors.Wrapf(err, "failed to convert value %v from event attribute to time.Time", value) } switch op { case OpLessEqual: - return v.Before(operandAsTime) || v.Equal(operandAsTime) + return (v.Before(operandAsTime) || v.Equal(operandAsTime)), nil case OpGreaterEqual: - return v.Equal(operandAsTime) || v.After(operandAsTime) + return (v.Equal(operandAsTime) || v.After(operandAsTime)), nil case OpLess: - return v.Before(operandAsTime) + return v.Before(operandAsTime), nil case OpGreater: - return v.After(operandAsTime) + return v.After(operandAsTime), nil case OpEqual: - return v.Equal(operandAsTime) + return v.Equal(operandAsTime), nil } case reflect.Float64: - operandFloat64 := operand.Interface().(float64) var v float64 + operandFloat64 := operand.Interface().(float64) + filteredValue := numRegex.FindString(value) + // try our best to convert value from tags to float64 - v, err := strconv.ParseFloat(value, 64) + v, err := strconv.ParseFloat(filteredValue, 64) if err != nil { - panic(fmt.Sprintf("failed to convert value %v from tag to float64: %v", value, err)) + return false, errors.Wrapf(err, "failed to convert value %v from event attribute to float64", filteredValue) } switch op { case OpLessEqual: - return v <= operandFloat64 + return v <= operandFloat64, nil case OpGreaterEqual: - return v >= operandFloat64 + return v >= operandFloat64, nil case OpLess: - return v < operandFloat64 + return v < operandFloat64, nil case OpGreater: - return v > operandFloat64 + return v > operandFloat64, nil case OpEqual: - return v == operandFloat64 + return v == operandFloat64, nil } case reflect.Int64: - operandInt := operand.Interface().(int64) var v int64 + + operandInt := operand.Interface().(int64) + filteredValue := numRegex.FindString(value) + // if value looks like float, we try to parse it as float - if strings.ContainsAny(value, ".") { - v1, err := strconv.ParseFloat(value, 64) + if strings.ContainsAny(filteredValue, ".") { + v1, err := strconv.ParseFloat(filteredValue, 64) if err != nil { - panic(fmt.Sprintf("failed to convert value %v from tag to float64: %v", value, err)) + return false, errors.Wrapf(err, "failed to convert value %v from event attribute to float64", filteredValue) } + v = int64(v1) } else { var err error // try our best to convert value from tags to int64 - v, err = strconv.ParseInt(value, 10, 64) + v, err = strconv.ParseInt(filteredValue, 10, 64) if err != nil { - panic(fmt.Sprintf("failed to convert value %v from tag to int64: %v", value, err)) + return false, errors.Wrapf(err, "failed to convert value %v from event attribute to int64", filteredValue) } } + switch op { case OpLessEqual: - return v <= operandInt + return v <= operandInt, nil case OpGreaterEqual: - return v >= operandInt + return v >= operandInt, nil case OpLess: - return v < operandInt + return v < operandInt, nil case OpGreater: - return v > operandInt + return v > operandInt, nil case OpEqual: - return v == operandInt + return v == operandInt, nil } case reflect.String: switch op { case OpEqual: - return value == operand.String() + return value == operand.String(), nil case OpContains: - return strings.Contains(value, operand.String()) + return strings.Contains(value, operand.String()), nil } default: - panic(fmt.Sprintf("unknown kind of operand %v", operand.Kind())) + return false, fmt.Errorf("unknown kind of operand %v", operand.Kind()) } - return false + return false, nil } diff --git a/libs/pubsub/query/query_test.go b/libs/pubsub/query/query_test.go index 17e7f6ebd..0572c74ff 100644 --- a/libs/pubsub/query/query_test.go +++ b/libs/pubsub/query/query_test.go @@ -18,84 +18,99 @@ func TestMatches(t *testing.T) { ) testCases := []struct { - s string - events map[string][]string - err bool - matches bool + s string + events map[string][]string + err bool + matches bool + matchErr bool }{ - {"tm.events.type='NewBlock'", map[string][]string{"tm.events.type": {"NewBlock"}}, false, true}, - - {"tx.gas > 7", map[string][]string{"tx.gas": {"8"}}, false, true}, - {"tx.gas > 7 AND tx.gas < 9", map[string][]string{"tx.gas": {"8"}}, false, true}, - {"body.weight >= 3.5", map[string][]string{"body.weight": {"3.5"}}, false, true}, - {"account.balance < 1000.0", map[string][]string{"account.balance": {"900"}}, false, true}, - {"apples.kg <= 4", map[string][]string{"apples.kg": {"4.0"}}, false, true}, - {"body.weight >= 4.5", map[string][]string{"body.weight": {fmt.Sprintf("%v", float32(4.5))}}, false, true}, + {"tm.events.type='NewBlock'", map[string][]string{"tm.events.type": {"NewBlock"}}, false, true, false}, + {"tx.gas > 7", map[string][]string{"tx.gas": {"8"}}, false, true, false}, + {"transfer.amount > 7", map[string][]string{"transfer.amount": {"8stake"}}, false, true, false}, + {"transfer.amount > 7", map[string][]string{"transfer.amount": {"8.045stake"}}, false, true, false}, + {"transfer.amount > 7.043", map[string][]string{"transfer.amount": {"8.045stake"}}, false, true, false}, + {"transfer.amount > 8.045", map[string][]string{"transfer.amount": {"8.045stake"}}, false, false, false}, + {"tx.gas > 7 AND tx.gas < 9", map[string][]string{"tx.gas": {"8"}}, false, true, false}, + {"body.weight >= 3.5", map[string][]string{"body.weight": {"3.5"}}, false, true, false}, + {"account.balance < 1000.0", map[string][]string{"account.balance": {"900"}}, false, true, false}, + {"apples.kg <= 4", map[string][]string{"apples.kg": {"4.0"}}, false, true, false}, + {"body.weight >= 4.5", map[string][]string{"body.weight": {fmt.Sprintf("%v", float32(4.5))}}, false, true, false}, { "oranges.kg < 4 AND watermellons.kg > 10", map[string][]string{"oranges.kg": {"3"}, "watermellons.kg": {"12"}}, false, true, + false, }, - {"peaches.kg < 4", map[string][]string{"peaches.kg": {"5"}}, false, false}, - + {"peaches.kg < 4", map[string][]string{"peaches.kg": {"5"}}, false, false, false}, { "tx.date > DATE 2017-01-01", map[string][]string{"tx.date": {time.Now().Format(query.DateLayout)}}, false, true, + false, }, - {"tx.date = DATE 2017-01-01", map[string][]string{"tx.date": {txDate}}, false, true}, - {"tx.date = DATE 2018-01-01", map[string][]string{"tx.date": {txDate}}, false, false}, - + {"tx.date = DATE 2017-01-01", map[string][]string{"tx.date": {txDate}}, false, true, false}, + {"tx.date = DATE 2018-01-01", map[string][]string{"tx.date": {txDate}}, false, false, false}, { "tx.time >= TIME 2013-05-03T14:45:00Z", map[string][]string{"tx.time": {time.Now().Format(query.TimeLayout)}}, false, true, + false, }, - {"tx.time = TIME 2013-05-03T14:45:00Z", map[string][]string{"tx.time": {txTime}}, false, false}, - - {"abci.owner.name CONTAINS 'Igor'", map[string][]string{"abci.owner.name": {"Igor,Ivan"}}, false, true}, - {"abci.owner.name CONTAINS 'Igor'", map[string][]string{"abci.owner.name": {"Pavel,Ivan"}}, false, false}, - - {"abci.owner.name = 'Igor'", map[string][]string{"abci.owner.name": {"Igor", "Ivan"}}, false, true}, + {"tx.time = TIME 2013-05-03T14:45:00Z", map[string][]string{"tx.time": {txTime}}, false, false, false}, + {"abci.owner.name CONTAINS 'Igor'", map[string][]string{"abci.owner.name": {"Igor,Ivan"}}, false, true, false}, + {"abci.owner.name CONTAINS 'Igor'", map[string][]string{"abci.owner.name": {"Pavel,Ivan"}}, false, false, false}, + {"abci.owner.name = 'Igor'", map[string][]string{"abci.owner.name": {"Igor", "Ivan"}}, false, true, false}, { "abci.owner.name = 'Ivan'", map[string][]string{"abci.owner.name": {"Igor", "Ivan"}}, false, true, + false, }, { "abci.owner.name = 'Ivan' AND abci.owner.name = 'Igor'", map[string][]string{"abci.owner.name": {"Igor", "Ivan"}}, false, true, + false, }, { "abci.owner.name = 'Ivan' AND abci.owner.name = 'John'", map[string][]string{"abci.owner.name": {"Igor", "Ivan"}}, false, false, + false, }, { "tm.events.type='NewBlock'", map[string][]string{"tm.events.type": {"NewBlock"}, "app.name": {"fuzzed"}}, false, true, + false, + }, + { + "app.name = 'fuzzed'", + map[string][]string{"tm.events.type": {"NewBlock"}, "app.name": {"fuzzed"}}, + false, + true, + false, }, - {"app.name = 'fuzzed'", map[string][]string{"tm.events.type": {"NewBlock"}, "app.name": {"fuzzed"}}, false, true}, { "tm.events.type='NewBlock' AND app.name = 'fuzzed'", map[string][]string{"tm.events.type": {"NewBlock"}, "app.name": {"fuzzed"}}, false, true, + false, }, { "tm.events.type='NewHeader' AND app.name = 'fuzzed'", map[string][]string{"tm.events.type": {"NewBlock"}, "app.name": {"fuzzed"}}, false, false, + false, }, } @@ -108,9 +123,13 @@ func TestMatches(t *testing.T) { require.NotNil(t, q, "Query '%s' should not be nil", tc.s) if tc.matches { - assert.True(t, q.Matches(tc.events), "Query '%s' should match %v", tc.s, tc.events) + match, err := q.Matches(tc.events) + assert.Nil(t, err, "Query '%s' should not error on match %v", tc.s, tc.events) + assert.True(t, match, "Query '%s' should match %v", tc.s, tc.events) } else { - assert.False(t, q.Matches(tc.events), "Query '%s' should not match %v", tc.s, tc.events) + match, err := q.Matches(tc.events) + assert.Equal(t, tc.matchErr, err != nil, "Unexpected error for query '%s' match %v", tc.s, tc.events) + assert.False(t, match, "Query '%s' should not match %v", tc.s, tc.events) } } } @@ -153,6 +172,8 @@ func TestConditions(t *testing.T) { q, err := query.New(tc.s) require.Nil(t, err) - assert.Equal(t, tc.conditions, q.Conditions()) + c, err := q.Conditions() + require.NoError(t, err) + assert.Equal(t, tc.conditions, c) } } diff --git a/state/txindex/kv/kv.go b/state/txindex/kv/kv.go index 802d13507..8fcce4862 100644 --- a/state/txindex/kv/kv.go +++ b/state/txindex/kv/kv.go @@ -168,7 +168,10 @@ func (txi *TxIndex) Search(q *query.Query) ([]*types.TxResult, error) { filteredHashes := make(map[string][]byte) // get a list of conditions (like "tx.height > 5") - conditions := q.Conditions() + conditions, err := q.Conditions() + if err != nil { + return nil, errors.Wrap(err, "error during parsing conditions from query") + } // if there is a hash condition, return the result immediately hash, err, ok := lookForHash(conditions)