mirror of
https://github.com/fiatjaf/khatru.git
synced 2025-03-26 17:51:49 +01:00
refactor(postgres): make SQL generation testable
Decouples the postgresql sql generation from the query execution. This allows the logic for building sql to be unit tested without access to a database. This work was motivated when a client was not receiving events as expected. In debugging I found that if a tag's value was an empty array, then no query would be executed - and to my surprised no error is raised either. I wanted to get a better sense of the current constraints on when queries are and are not executed, but I had a hard time keeping the code in my head. This led me to extracting the sql generation into its own function and writing the unit tests that document its current behavior. This refactor makes no changes to the current logic. I have added some REVIEW comments in the test cases where I thought some error handling could be introduced but I wanted to first see if you were receptive to this refactor before proposing any functional changes.
This commit is contained in:
bndw
fiatjaf_
parent
e84f5df1f0
commit
ae641fd24d
5
go.mod
5
go.mod
@ -18,7 +18,9 @@ require (
|
||||
github.com/mmcdole/gofeed v1.1.3
|
||||
github.com/nbd-wtf/go-nostr v0.17.1
|
||||
github.com/rif/cache2go v1.0.0
|
||||
github.com/rs/cors v1.7.0
|
||||
github.com/stevelacy/daz v0.1.4
|
||||
github.com/stretchr/testify v1.8.0
|
||||
github.com/tidwall/gjson v1.14.1
|
||||
golang.org/x/exp v0.0.0-20230224173230-c95f2b4c22f2
|
||||
)
|
||||
@ -79,7 +81,7 @@ require (
|
||||
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
|
||||
github.com/modern-go/reflect2 v1.0.1 // indirect
|
||||
github.com/pkg/errors v0.9.1 // indirect
|
||||
github.com/rs/cors v1.7.0 // indirect
|
||||
github.com/pmezard/go-difflib v1.0.0 // indirect
|
||||
github.com/tidwall/match v1.1.1 // indirect
|
||||
github.com/tidwall/pretty v1.2.0 // indirect
|
||||
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 // indirect
|
||||
@ -87,4 +89,5 @@ require (
|
||||
golang.org/x/sys v0.1.0 // indirect
|
||||
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
|
||||
golang.org/x/text v0.3.7 // indirect
|
||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
||||
)
|
||||
|
||||
6
go.sum
6
go.sum
@ -394,7 +394,8 @@ github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DM
|
||||
github.com/stevelacy/daz v0.1.4 h1:ugmff/D7D764wZjXSgSryEINE/bi+Xddllw3JQQGbWk=
|
||||
github.com/stevelacy/daz v0.1.4/go.mod h1:AbK6DzjiIL15r4bQtcFvOBAvDGMXoh+uIG26NRUugt0=
|
||||
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
|
||||
github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
|
||||
github.com/stretchr/objx v0.4.0 h1:M2gUjqZET1qApGOWNSnZ49BAIMX4F/1plDv3+l31EJ4=
|
||||
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
|
||||
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
|
||||
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
|
||||
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
|
||||
@ -402,6 +403,7 @@ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5
|
||||
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
|
||||
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
|
||||
github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
|
||||
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
|
||||
github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 h1:epCh84lMvA70Z7CTTCmYQn2CKbY8j86K7/FAIr141uY=
|
||||
github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7/go.mod h1:q4W45IWZaF22tdD+VEXcAWRA037jwmWEB5VWYORlTpc=
|
||||
github.com/tidwall/gjson v1.14.1 h1:iymTbGkQBhveq21bEvAQ81I0LEBork8BFe1CUZXdyuo=
|
||||
@ -609,6 +611,7 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8
|
||||
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
|
||||
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
|
||||
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
|
||||
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
|
||||
gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
|
||||
gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE=
|
||||
gopkg.in/go-playground/validator.v8 v8.18.2/go.mod h1:RX2a/7Ha8BgOhfk7j780h4/u/RRjR0eouCJSH80/M2Y=
|
||||
@ -622,6 +625,7 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
|
||||
gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
|
||||
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
|
||||
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
|
||||
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
|
||||
honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
|
||||
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
|
||||
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
|
||||
|
||||
@ -4,29 +4,58 @@ import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/jmoiron/sqlx"
|
||||
"github.com/nbd-wtf/go-nostr"
|
||||
)
|
||||
|
||||
func (b PostgresBackend) QueryEvents(ctx context.Context, filter *nostr.Filter) (ch chan *nostr.Event, err error) {
|
||||
ch = make(chan *nostr.Event)
|
||||
|
||||
query, params, err := queryEventsSql(filter)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
rows, err := b.DB.Query(query, params...)
|
||||
if err != nil && err != sql.ErrNoRows {
|
||||
return nil, fmt.Errorf("failed to fetch events using query %q: %w", query, err)
|
||||
}
|
||||
|
||||
go func() {
|
||||
defer rows.Close()
|
||||
defer close(ch)
|
||||
for rows.Next() {
|
||||
var evt nostr.Event
|
||||
var timestamp int64
|
||||
err := rows.Scan(&evt.ID, &evt.PubKey, ×tamp,
|
||||
&evt.Kind, &evt.Tags, &evt.Content, &evt.Sig)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
evt.CreatedAt = nostr.Timestamp(timestamp)
|
||||
ch <- &evt
|
||||
}
|
||||
}()
|
||||
|
||||
return ch, nil
|
||||
}
|
||||
|
||||
func queryEventsSql(filter *nostr.Filter) (string, []any, error) {
|
||||
var conditions []string
|
||||
var params []any
|
||||
|
||||
if filter == nil {
|
||||
err = errors.New("filter cannot be null")
|
||||
return
|
||||
return "", nil, fmt.Errorf("filter cannot be null")
|
||||
}
|
||||
|
||||
if filter.IDs != nil {
|
||||
if len(filter.IDs) > 500 {
|
||||
// too many ids, fail everything
|
||||
return
|
||||
return "", nil, nil
|
||||
}
|
||||
|
||||
likeids := make([]string, 0, len(filter.IDs))
|
||||
@ -41,7 +70,7 @@ func (b PostgresBackend) QueryEvents(ctx context.Context, filter *nostr.Filter)
|
||||
}
|
||||
if len(likeids) == 0 {
|
||||
// ids being [] mean you won't get anything
|
||||
return
|
||||
return "", nil, nil
|
||||
}
|
||||
conditions = append(conditions, "("+strings.Join(likeids, " OR ")+")")
|
||||
}
|
||||
@ -49,7 +78,7 @@ func (b PostgresBackend) QueryEvents(ctx context.Context, filter *nostr.Filter)
|
||||
if filter.Authors != nil {
|
||||
if len(filter.Authors) > 500 {
|
||||
// too many authors, fail everything
|
||||
return
|
||||
return "", nil, nil
|
||||
}
|
||||
|
||||
likekeys := make([]string, 0, len(filter.Authors))
|
||||
@ -64,7 +93,7 @@ func (b PostgresBackend) QueryEvents(ctx context.Context, filter *nostr.Filter)
|
||||
}
|
||||
if len(likekeys) == 0 {
|
||||
// authors being [] mean you won't get anything
|
||||
return
|
||||
return "", nil, nil
|
||||
}
|
||||
conditions = append(conditions, "("+strings.Join(likekeys, " OR ")+")")
|
||||
}
|
||||
@ -72,12 +101,12 @@ func (b PostgresBackend) QueryEvents(ctx context.Context, filter *nostr.Filter)
|
||||
if filter.Kinds != nil {
|
||||
if len(filter.Kinds) > 10 {
|
||||
// too many kinds, fail everything
|
||||
return
|
||||
return "", nil, nil
|
||||
}
|
||||
|
||||
if len(filter.Kinds) == 0 {
|
||||
// kinds being [] mean you won't get anything
|
||||
return
|
||||
return "", nil, nil
|
||||
}
|
||||
// no sql injection issues since these are ints
|
||||
inkinds := make([]string, len(filter.Kinds))
|
||||
@ -91,7 +120,7 @@ func (b PostgresBackend) QueryEvents(ctx context.Context, filter *nostr.Filter)
|
||||
for _, values := range filter.Tags {
|
||||
if len(values) == 0 {
|
||||
// any tag set to [] is wrong
|
||||
return
|
||||
return "", nil, nil
|
||||
}
|
||||
|
||||
// add these tags to the query
|
||||
@ -99,7 +128,7 @@ func (b PostgresBackend) QueryEvents(ctx context.Context, filter *nostr.Filter)
|
||||
|
||||
if len(tagQuery) > 10 {
|
||||
// too many tags, fail everything
|
||||
return
|
||||
return "", nil, nil
|
||||
}
|
||||
}
|
||||
|
||||
@ -136,32 +165,11 @@ func (b PostgresBackend) QueryEvents(ctx context.Context, filter *nostr.Filter)
|
||||
params = append(params, filter.Limit)
|
||||
}
|
||||
|
||||
query := b.DB.Rebind(`SELECT
|
||||
query := sqlx.Rebind(sqlx.BindType("postgres"), `SELECT
|
||||
id, pubkey, created_at, kind, tags, content, sig
|
||||
FROM event WHERE ` +
|
||||
strings.Join(conditions, " AND ") +
|
||||
FROM event WHERE `+
|
||||
strings.Join(conditions, " AND ")+
|
||||
" ORDER BY created_at DESC LIMIT ?")
|
||||
|
||||
rows, err := b.DB.Query(query, params...)
|
||||
if err != nil && err != sql.ErrNoRows {
|
||||
return nil, fmt.Errorf("failed to fetch events using query %q: %w", query, err)
|
||||
}
|
||||
|
||||
go func() {
|
||||
defer rows.Close()
|
||||
defer close(ch)
|
||||
for rows.Next() {
|
||||
var evt nostr.Event
|
||||
var timestamp int64
|
||||
err := rows.Scan(&evt.ID, &evt.PubKey, ×tamp,
|
||||
&evt.Kind, &evt.Tags, &evt.Content, &evt.Sig)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
evt.CreatedAt = nostr.Timestamp(timestamp)
|
||||
ch <- &evt
|
||||
}
|
||||
}()
|
||||
|
||||
return ch, nil
|
||||
return query, params, nil
|
||||
}
|
||||
|
||||
190
storage/postgresql/query_test.go
Normal file
190
storage/postgresql/query_test.go
Normal file
@ -0,0 +1,190 @@
|
||||
package postgresql
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/nbd-wtf/go-nostr"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestQueryEventsSql(t *testing.T) {
|
||||
var tests = []struct {
|
||||
name string
|
||||
filter *nostr.Filter
|
||||
query string
|
||||
params []any
|
||||
err error
|
||||
}{
|
||||
{
|
||||
name: "empty filter",
|
||||
filter: &nostr.Filter{},
|
||||
query: "SELECT id, pubkey, created_at, kind, tags, content, sig FROM event WHERE true ORDER BY created_at DESC LIMIT $1",
|
||||
params: []any{100},
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "ids filter",
|
||||
filter: &nostr.Filter{
|
||||
IDs: []string{"083ec57f36a7b39ab98a57bedab4f85355b2ee89e4b205bed58d7c3ef9edd294"},
|
||||
},
|
||||
query: `SELECT id, pubkey, created_at, kind, tags, content, sig
|
||||
FROM event
|
||||
WHERE (id LIKE '083ec57f36a7b39ab98a57bedab4f85355b2ee89e4b205bed58d7c3ef9edd294%')
|
||||
ORDER BY created_at DESC LIMIT $1`,
|
||||
params: []any{100},
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "kind filter",
|
||||
filter: &nostr.Filter{
|
||||
Kinds: []int{1, 2, 3},
|
||||
},
|
||||
query: `SELECT id, pubkey, created_at, kind, tags, content, sig
|
||||
FROM event
|
||||
WHERE kind IN(1,2,3)
|
||||
ORDER BY created_at DESC LIMIT $1`,
|
||||
params: []any{100},
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "authors filter",
|
||||
filter: &nostr.Filter{
|
||||
Authors: []string{"7bdef7bdebb8721f77927d0e77c66059360fa62371fdf15f3add93923a613229"},
|
||||
},
|
||||
query: `SELECT id, pubkey, created_at, kind, tags, content, sig
|
||||
FROM event
|
||||
WHERE (pubkey LIKE '7bdef7bdebb8721f77927d0e77c66059360fa62371fdf15f3add93923a613229%')
|
||||
ORDER BY created_at DESC LIMIT $1`,
|
||||
params: []any{100},
|
||||
err: nil,
|
||||
},
|
||||
// errors
|
||||
{
|
||||
name: "nil filter",
|
||||
filter: nil,
|
||||
query: "",
|
||||
params: nil,
|
||||
err: fmt.Errorf("filter cannot be null"),
|
||||
},
|
||||
{
|
||||
name: "too many ids",
|
||||
filter: &nostr.Filter{
|
||||
IDs: strSlice(501),
|
||||
},
|
||||
query: "",
|
||||
params: nil,
|
||||
// REVIEW: should return error
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "invalid ids",
|
||||
filter: &nostr.Filter{
|
||||
IDs: []string{"stuff"},
|
||||
},
|
||||
query: "",
|
||||
params: nil,
|
||||
// REVIEW: should return error
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "too many authors",
|
||||
filter: &nostr.Filter{
|
||||
Authors: strSlice(501),
|
||||
},
|
||||
query: "",
|
||||
params: nil,
|
||||
// REVIEW: should return error
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "invalid authors",
|
||||
filter: &nostr.Filter{
|
||||
Authors: []string{"stuff"},
|
||||
},
|
||||
query: "",
|
||||
params: nil,
|
||||
// REVIEW: should return error
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "too many kinds",
|
||||
filter: &nostr.Filter{
|
||||
Kinds: intSlice(11),
|
||||
},
|
||||
query: "",
|
||||
params: nil,
|
||||
// REVIEW: should return error
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "no kinds",
|
||||
filter: &nostr.Filter{
|
||||
Kinds: []int{},
|
||||
},
|
||||
query: "",
|
||||
params: nil,
|
||||
// REVIEW: should return error
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "tags of empty array",
|
||||
filter: &nostr.Filter{
|
||||
Tags: nostr.TagMap{
|
||||
"#e": []string{},
|
||||
},
|
||||
},
|
||||
query: "",
|
||||
params: nil,
|
||||
// REVIEW: should return error
|
||||
err: nil,
|
||||
},
|
||||
{
|
||||
name: "too many tag values",
|
||||
filter: &nostr.Filter{
|
||||
Tags: nostr.TagMap{
|
||||
"#e": strSlice(11),
|
||||
},
|
||||
},
|
||||
query: "",
|
||||
params: nil,
|
||||
// REVIEW: should return error
|
||||
err: nil,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
query, params, err := queryEventsSql(tt.filter)
|
||||
assert.Equal(t, tt.err, err)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
assert.Equal(t, clean(tt.query), clean(query))
|
||||
assert.Equal(t, tt.params, params)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func clean(s string) string {
|
||||
return strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(s, "\t", ""), "\n", ""), " ", "")
|
||||
}
|
||||
|
||||
func intSlice(n int) []int {
|
||||
slice := make([]int, 0, n)
|
||||
for i := 0; i < n; i++ {
|
||||
slice = append(slice, i)
|
||||
}
|
||||
return slice
|
||||
}
|
||||
|
||||
func strSlice(n int) []string {
|
||||
slice := make([]string, 0, n)
|
||||
for i := 0; i < n; i++ {
|
||||
slice = append(slice, strconv.Itoa(i))
|
||||
}
|
||||
return slice
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user