From af7c642f963b58ccfd763b495f33e6bb7c27b7f4 Mon Sep 17 00:00:00 2001 From: Andrei Mihu Date: Thu, 9 Jan 2020 11:52:47 +0000 Subject: [PATCH] Ensure tournament listing correctly uses the cursor on paginated requests. --- CHANGELOG.md | 3 + server/api_tournament.go | 6 +- server/core_tournament.go | 115 ++++++++++++------------ server/leaderboard_cache.go | 164 ++++++++++++++++++++++++++++++----- server/runtime_go_nakama.go | 6 +- server/runtime_lua_nakama.go | 6 +- 6 files changed, 213 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4076ca900..6eb1b6f92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ The format is based on [keep a changelog](http://keepachangelog.com) and this pr ### Changed - Upgrade devconsole handlebars (4.3.0) dependency. +### Fixed +- Ensure tournament listing correctly uses the cursor on paginated requests. + ## [2.9.0] - 2019-12-23 ### Added - New runtime functions to retrieve tournaments by ID. diff --git a/server/api_tournament.go b/server/api_tournament.go index 94a545bf7..5ba1a659b 100644 --- a/server/api_tournament.go +++ b/server/api_tournament.go @@ -199,14 +199,14 @@ func (s *ApiServer) ListTournaments(ctx context.Context, in *api.ListTournaments } } - var incomingCursor *tournamentListCursor + var incomingCursor *TournamentListCursor if in.GetCursor() != "" { cb, err := base64.StdEncoding.DecodeString(in.GetCursor()) if err != nil { return nil, ErrLeaderboardInvalidCursor } - incomingCursor = &tournamentListCursor{} + incomingCursor = &TournamentListCursor{} if err := gob.NewDecoder(bytes.NewReader(cb)).Decode(incomingCursor); err != nil { return nil, ErrLeaderboardInvalidCursor } @@ -249,7 +249,7 @@ func (s *ApiServer) ListTournaments(ctx context.Context, in *api.ListTournaments } } - records, err := TournamentList(ctx, s.logger, s.db, categoryStart, categoryEnd, startTime, endTime, limit, incomingCursor) + records, err := TournamentList(ctx, s.logger, s.db, s.leaderboardCache, categoryStart, categoryEnd, startTime, endTime, limit, incomingCursor) if err != nil { return nil, status.Error(codes.Internal, "Error listing tournaments.") } diff --git a/server/core_tournament.go b/server/core_tournament.go index 39602d2b1..097a7f609 100644 --- a/server/core_tournament.go +++ b/server/core_tournament.go @@ -22,6 +22,7 @@ import ( "encoding/gob" "errors" "fmt" + "strconv" "strings" "time" @@ -42,9 +43,8 @@ var ( ErrTournamentWriteJoinRequired = errors.New("required to join before writing tournament record") ) -type tournamentListCursor struct { - // ID fields. - TournamentId string +type TournamentListCursor struct { + Id string } func TournamentCreate(ctx context.Context, logger *zap.Logger, cache LeaderboardCache, scheduler LeaderboardScheduler, leaderboardId string, sortOrder, operator int, resetSchedule, metadata, @@ -234,53 +234,30 @@ WHERE id IN (` + strings.Join(statements, ",") + `) AND duration > 0` return records, nil } -func TournamentList(ctx context.Context, logger *zap.Logger, db *sql.DB, categoryStart, categoryEnd, startTime, endTime, limit int, cursor *tournamentListCursor) (*api.TournamentList, error) { +func TournamentList(ctx context.Context, logger *zap.Logger, db *sql.DB, leaderboardCache LeaderboardCache, categoryStart, categoryEnd, startTime, endTime, limit int, cursor *TournamentListCursor) (*api.TournamentList, error) { now := time.Now().UTC() + nowUnix := now.Unix() - query := `SELECT id, sort_order, reset_schedule, metadata, create_time, category, description, duration, end_time, max_size, max_num_score, title, size, start_time -FROM leaderboard -WHERE duration > 0 AND category >= $1 AND category <= $2 AND start_time >= $3` - params := make([]interface{}, 0, 6) - if categoryStart >= 0 && categoryStart <= 127 { - params = append(params, categoryStart) - } else { - params = append(params, 0) - } - if categoryEnd >= 0 && categoryEnd <= 127 { - params = append(params, categoryEnd) - } else { - params = append(params, 127) - } - - if startTime >= 0 { - params = append(params, time.Unix(int64(startTime), 0).UTC()) - } else { - params = append(params, time.Unix(0, 0).UTC()) + list, newCursor, err := leaderboardCache.ListTournaments(nowUnix, categoryStart, categoryEnd, int64(startTime), int64(endTime), limit, cursor) + if err != nil { + logger.Error("Could not retrieve tournaments", zap.Error(err)) + return nil, err } - if endTime == 0 { - query += " AND end_time = $4" - params = append(params, time.Unix(0, 0).UTC()) - } else if int64(endTime) < now.Unix() { - // if end time is set explicitly in the past - query += " AND end_time <= $4" - params = append(params, time.Unix(int64(endTime), 0).UTC()) - } else { - // if end time is in the future, return both tournaments that end in the future as well as the ones that never end. - query += " AND (end_time <= $4 OR end_time = '1970-01-01 00:00:00 UTC')" - params = append(params, time.Unix(int64(endTime), 0).UTC()) + if len(list) == 0 { + return &api.TournamentList{ + Tournaments: []*api.Tournament{}, + }, nil } - // To ensure that there are more records, so the cursor is returned - params = append(params, limit+1) - - if cursor != nil { - query += " AND id > $6" - params = append(params, cursor.TournamentId) + // Read most up to date sizes from database. + statements := make([]string, 0, len(list)) + params := make([]interface{}, 0, len(list)) + for i, leaderboard := range list { + params = append(params, leaderboard.Id) + statements = append(statements, "$"+strconv.Itoa(i+1)) } - - query += " LIMIT $5" - + query := "SELECT id, size FROM leaderboard WHERE id IN (" + strings.Join(statements, ",") + ")" logger.Debug("Tournament listing query", zap.String("query", query), zap.Any("params", params)) rows, err := db.QueryContext(ctx, query, params...) if err != nil { @@ -288,32 +265,56 @@ WHERE duration > 0 AND category >= $1 AND category <= $2 AND start_time >= $3` return nil, err } - records := make([]*api.Tournament, 0) - newCursor := &tournamentListCursor{} - - count := 0 + sizes := make(map[string]int, len(list)) + var dbID string + var dbSize int for rows.Next() { - tournament, err := parseTournament(rows, now) - if err != nil { + if err := rows.Scan(&dbID, &dbSize); err != nil { _ = rows.Close() logger.Error("Error parsing listed tournament records", zap.Error(err)) return nil, err } - count++ + sizes[dbID] = dbSize + } + _ = rows.Close() - if count <= limit { - records = append(records, tournament) - } else if count > limit { - newCursor.TournamentId = records[limit-1].Id + records := make([]*api.Tournament, 0, len(list)) + for _, leaderboard := range list { + size := sizes[leaderboard.Id] + startActive, endActiveUnix, expiryUnix := calculateTournamentDeadlines(leaderboard.StartTime, leaderboard.EndTime, int64(leaderboard.Duration), leaderboard.ResetSchedule, now) + canEnter := true + + if startActive > nowUnix || endActiveUnix < nowUnix { + canEnter = false + } + if canEnter && size >= leaderboard.MaxSize { + canEnter = false } + + records = append(records, &api.Tournament{ + Id: leaderboard.Id, + Title: leaderboard.Title, + Description: leaderboard.Description, + Category: uint32(leaderboard.Category), + SortOrder: uint32(leaderboard.SortOrder), + Size: uint32(size), + MaxSize: uint32(leaderboard.MaxSize), + MaxNumScore: uint32(leaderboard.MaxNumScore), + CanEnter: canEnter, + EndActive: uint32(endActiveUnix), + NextReset: uint32(expiryUnix), + Metadata: leaderboard.Metadata, + CreateTime: ×tamp.Timestamp{Seconds: leaderboard.CreateTime}, + StartTime: ×tamp.Timestamp{Seconds: leaderboard.StartTime}, + Duration: uint32(leaderboard.Duration), + StartActive: uint32(startActive), + }) } - _ = rows.Close() tournamentList := &api.TournamentList{ Tournaments: records, } - - if newCursor.TournamentId != "" { + if newCursor != nil { cursorBuf := new(bytes.Buffer) if err := gob.NewEncoder(cursorBuf).Encode(newCursor); err != nil { logger.Error("Error creating tournament records list cursor", zap.Error(err)) diff --git a/server/leaderboard_cache.go b/server/leaderboard_cache.go index 04241f16c..039a90991 100644 --- a/server/leaderboard_cache.go +++ b/server/leaderboard_cache.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "log" + "sort" "strconv" "sync" "time" @@ -109,6 +110,34 @@ func (l *Leaderboard) GetCreateTime() int64 { return l.CreateTime } +// Type alias for a list of tournaments that binds together sorting functions. +// Not intended to be used for sorting lists of non-tournament leaderboards. +type OrderedTournaments []*Leaderboard + +func (t OrderedTournaments) Len() int { + return len(t) +} +func (t OrderedTournaments) Swap(i, j int) { + t[i], t[j] = t[j], t[i] +} +func (t OrderedTournaments) Less(i, j int) bool { + ti, tj := t[i], t[j] + if ti.StartTime < tj.StartTime { + return true + } else if ti.StartTime == tj.StartTime { + if ti.EndTime > tj.EndTime { + return true + } else if ti.EndTime == tj.EndTime { + if ti.Category < tj.Category { + return true + } else if ti.Category == tj.Category { + return ti.Id < tj.Id + } + } + } + return false +} + type LeaderboardCache interface { Get(id string) *Leaderboard GetAllLeaderboards() []*Leaderboard @@ -117,6 +146,7 @@ type LeaderboardCache interface { Insert(id string, authoritative bool, sortOrder, operator int, resetSchedule, metadata string, createTime int64) CreateTournament(ctx context.Context, id string, sortOrder, operator int, resetSchedule, metadata, title, description string, category, startTime, endTime, duration, maxSize, maxNumScore int, joinRequired bool) (*Leaderboard, error) InsertTournament(id string, sortOrder, operator int, resetSchedule, metadata, title, description string, category, duration, maxSize, maxNumScore int, joinRequired bool, createTime, startTime, endTime int64) + ListTournaments(now int64, categoryStart, categoryEnd int, startTime, endTime int64, limit int, cursor *TournamentListCursor) ([]*Leaderboard, *TournamentListCursor, error) Delete(ctx context.Context, id string) error Remove(id string) } @@ -126,6 +156,8 @@ type LocalLeaderboardCache struct { logger *zap.Logger db *sql.DB leaderboards map[string]*Leaderboard + + tournamentList []*Leaderboard } func NewLocalLeaderboardCache(logger, startupLogger *zap.Logger, db *sql.DB) LeaderboardCache { @@ -133,6 +165,8 @@ func NewLocalLeaderboardCache(logger, startupLogger *zap.Logger, db *sql.DB) Lea logger: logger, db: db, leaderboards: make(map[string]*Leaderboard), + + tournamentList: make([]*Leaderboard, 0), } err := l.RefreshAllLeaderboards(context.Background()) @@ -157,6 +191,7 @@ FROM leaderboard` } leaderboards := make(map[string]*Leaderboard) + tournamentList := make([]*Leaderboard, 0) for rows.Next() { var id string @@ -217,11 +252,18 @@ FROM leaderboard` } leaderboards[id] = leaderboard + + if leaderboard.IsTournament() { + tournamentList = append(tournamentList, leaderboard) + } } _ = rows.Close() + sort.Sort(OrderedTournaments(tournamentList)) + l.Lock() l.leaderboards = leaderboards + l.tournamentList = tournamentList l.Unlock() return nil @@ -252,6 +294,7 @@ func (l *LocalLeaderboardCache) Create(ctx context.Context, id string, authorita l.Unlock() return leaderboard, nil } + l.Unlock() var expr *cronexpr.Expression var err error @@ -280,7 +323,6 @@ func (l *LocalLeaderboardCache) Create(ctx context.Context, id string, authorita var createTime pgtype.Timestamptz err = l.db.QueryRowContext(ctx, query, params...).Scan(&createTime) if err != nil { - l.Unlock() l.logger.Error("Error creating leaderboard", zap.Error(err)) return nil, err } @@ -296,8 +338,14 @@ func (l *LocalLeaderboardCache) Create(ctx context.Context, id string, authorita Metadata: metadata, CreateTime: createTime.Time.Unix(), } - l.leaderboards[id] = leaderboard + l.Lock() + if leaderboard, ok := l.leaderboards[id]; ok { + // Maybe multiple concurrent creations for this ID. + l.Unlock() + return leaderboard, nil + } + l.leaderboards[id] = leaderboard l.Unlock() return leaderboard, nil @@ -315,8 +363,7 @@ func (l *LocalLeaderboardCache) Insert(id string, authoritative bool, sortOrder, } } - l.Lock() - l.leaderboards[id] = &Leaderboard{ + leaderboard := &Leaderboard{ Id: id, Authoritative: authoritative, SortOrder: sortOrder, @@ -326,11 +373,15 @@ func (l *LocalLeaderboardCache) Insert(id string, authoritative bool, sortOrder, Metadata: metadata, CreateTime: createTime, } + + l.Lock() + l.leaderboards[id] = leaderboard l.Unlock() } func (l *LocalLeaderboardCache) CreateTournament(ctx context.Context, id string, sortOrder, operator int, resetSchedule, metadata, title, description string, category, startTime, endTime, duration, maxSize, maxNumScore int, joinRequired bool) (*Leaderboard, error) { - if err := checkTournamentConfig(resetSchedule, startTime, endTime, duration, maxSize, maxNumScore); err != nil { + resetCron, err := checkTournamentConfig(resetSchedule, startTime, endTime, duration, maxSize, maxNumScore) + if err != nil { l.logger.Error("Error while creating tournament", zap.Error(err)) return nil, err } @@ -433,20 +484,19 @@ func (l *LocalLeaderboardCache) CreateTournament(ctx context.Context, id string, var createTime pgtype.Timestamptz var dbStartTime pgtype.Timestamptz var dbEndTime pgtype.Timestamptz - err := l.db.QueryRowContext(ctx, query, params...).Scan(&createTime, &dbStartTime, &dbEndTime) + err = l.db.QueryRowContext(ctx, query, params...).Scan(&createTime, &dbStartTime, &dbEndTime) if err != nil { l.logger.Error("Error creating tournament", zap.Error(err)) return nil, err } - cron, _ := cronexpr.Parse(resetSchedule) leaderboard = &Leaderboard{ Id: id, Authoritative: true, SortOrder: sortOrder, Operator: operator, ResetScheduleStr: resetSchedule, - ResetSchedule: cron, + ResetSchedule: resetCron, Metadata: metadata, CreateTime: createTime.Time.Unix(), Category: category, @@ -465,6 +515,8 @@ func (l *LocalLeaderboardCache) CreateTournament(ctx context.Context, id string, l.Lock() l.leaderboards[id] = leaderboard + l.tournamentList = append(l.tournamentList, leaderboard) + sort.Sort(OrderedTournaments(l.tournamentList)) l.Unlock() return leaderboard, nil @@ -482,8 +534,7 @@ func (l *LocalLeaderboardCache) InsertTournament(id string, sortOrder, operator } } - l.Lock() - l.leaderboards[id] = &Leaderboard{ + leaderboard := &Leaderboard{ Id: id, Authoritative: true, SortOrder: sortOrder, @@ -502,12 +553,65 @@ func (l *LocalLeaderboardCache) InsertTournament(id string, sortOrder, operator StartTime: startTime, EndTime: endTime, } + + l.Lock() + l.leaderboards[id] = leaderboard + l.tournamentList = append(l.tournamentList, leaderboard) + sort.Sort(OrderedTournaments(l.tournamentList)) l.Unlock() } +func (l *LocalLeaderboardCache) ListTournaments(now int64, categoryStart, categoryEnd int, startTime, endTime int64, limit int, cursor *TournamentListCursor) ([]*Leaderboard, *TournamentListCursor, error) { + list := make([]*Leaderboard, 0, limit) + var newCursor *TournamentListCursor + skip := cursor != nil + + l.RLock() + for _, leaderboard := range l.tournamentList { + if skip { + if leaderboard.Id == cursor.Id { + skip = false + } + continue + } + + if leaderboard.Category < categoryStart { + // Skip tournaments with category before start boundary. + continue + } + if leaderboard.Category > categoryEnd { + // Skip tournaments with category after end boundary. + continue + } + if leaderboard.StartTime < startTime { + // Skip tournaments with start time before filter. + continue + } + if endTime == 0 && leaderboard.EndTime != 0 || (endTime < now && (leaderboard.EndTime > endTime || leaderboard.EndTime == 0)) || leaderboard.EndTime > endTime { + // SKIP tournaments where: + // - If end time filter is == 0, tournament end time is non-0. + // - If end time filter is in the past, tournament end time is after the filter or 0 (never end). + // - If end time is in the future, tournament end time is after the filter. + continue + } + + if ln := len(list); ln >= limit { + newCursor = &TournamentListCursor{ + Id: list[ln-1].Id, + } + break + } + + list = append(list, leaderboard) + } + l.RUnlock() + + return list, newCursor, nil +} + func (l *LocalLeaderboardCache) Delete(ctx context.Context, id string) error { l.Lock() - _, leaderboardFound := l.leaderboards[id] + leaderboard, leaderboardFound := l.leaderboards[id] l.Unlock() if !leaderboardFound { @@ -526,42 +630,60 @@ func (l *LocalLeaderboardCache) Delete(ctx context.Context, id string) error { l.Lock() // Then delete from cache. delete(l.leaderboards, id) + if leaderboard.IsTournament() { + for i, currentLeaderboard := range l.tournamentList { + if currentLeaderboard.Id == id { + l.tournamentList = append(l.tournamentList[:i], l.tournamentList[i+1:]...) + break + } + } + } l.Unlock() return nil } func (l *LocalLeaderboardCache) Remove(id string) { l.Lock() - delete(l.leaderboards, id) + if leaderboard, ok := l.leaderboards[id]; ok { + delete(l.leaderboards, id) + if leaderboard.IsTournament() { + for i, currentLeaderboard := range l.tournamentList { + if currentLeaderboard.Id == id { + l.tournamentList = append(l.tournamentList[:i], l.tournamentList[i+1:]...) + break + } + } + } + } l.Unlock() } -func checkTournamentConfig(resetSchedule string, startTime, endTime, duration, maxSize, maxNumScore int) error { +func checkTournamentConfig(resetSchedule string, startTime, endTime, duration, maxSize, maxNumScore int) (*cronexpr.Expression, error) { if startTime < 0 { - return fmt.Errorf("tournament start time must be a unix UTC time in the future") + return nil, fmt.Errorf("tournament start time must be a unix UTC time in the future") } if duration <= 0 { - return fmt.Errorf("tournament duration must be greater than zero") + return nil, fmt.Errorf("tournament duration must be greater than zero") } if maxSize < 0 { - return fmt.Errorf("tournament max must be greater than zero") + return nil, fmt.Errorf("tournament max must be greater than zero") } if maxNumScore < 0 { - return fmt.Errorf("tournament m num score must be greater than zero") + return nil, fmt.Errorf("tournament m num score must be greater than zero") } if (endTime > 0) && (endTime < startTime) { - return fmt.Errorf("tournament end time cannot be before start time") + return nil, fmt.Errorf("tournament end time cannot be before start time") } var cron *cronexpr.Expression if resetSchedule != "" { expr, err := cronexpr.Parse(resetSchedule) if err != nil { - return fmt.Errorf("could not parse reset schedule: %s", err.Error()) + return nil, fmt.Errorf("could not parse reset schedule: %s", err.Error()) } cron = expr } @@ -572,9 +694,9 @@ func checkTournamentConfig(resetSchedule string, startTime, endTime, duration, m // Check that the end time (if specified) is at least strictly after the first active period start time. if (endTime > 0) && (int64(endTime) <= firstResetUnix) { - return fmt.Errorf("tournament end time cannot be before first reset schedule - either increase end time or change/disable reset schedule") + return nil, fmt.Errorf("tournament end time cannot be before first reset schedule - either increase end time or change/disable reset schedule") } } - return nil + return cron, nil } diff --git a/server/runtime_go_nakama.go b/server/runtime_go_nakama.go index b02542926..5b13b6429 100644 --- a/server/runtime_go_nakama.go +++ b/server/runtime_go_nakama.go @@ -1473,19 +1473,19 @@ func (n *RuntimeGoNakamaModule) TournamentList(ctx context.Context, categoryStar return nil, errors.New("limit must be 1-100") } - var cursorPtr *tournamentListCursor + var cursorPtr *TournamentListCursor if cursor != "" { cb, err := base64.StdEncoding.DecodeString(cursor) if err != nil { return nil, errors.New("expects cursor to be valid when provided") } - cursorPtr = &tournamentListCursor{} + cursorPtr = &TournamentListCursor{} if err := gob.NewDecoder(bytes.NewReader(cb)).Decode(cursorPtr); err != nil { return nil, errors.New("expects cursor to be valid when provided") } } - return TournamentList(ctx, n.logger, n.db, categoryStart, categoryEnd, startTime, endTime, limit, cursorPtr) + return TournamentList(ctx, n.logger, n.db, n.leaderboardCache, categoryStart, categoryEnd, startTime, endTime, limit, cursorPtr) } func (n *RuntimeGoNakamaModule) TournamentRecordWrite(ctx context.Context, id, ownerID, username string, score, subscore int64, metadata map[string]interface{}) (*api.LeaderboardRecord, error) { diff --git a/server/runtime_lua_nakama.go b/server/runtime_lua_nakama.go index 92ace212d..560a3e6ee 100644 --- a/server/runtime_lua_nakama.go +++ b/server/runtime_lua_nakama.go @@ -4721,7 +4721,7 @@ func (n *RuntimeLuaNakamaModule) tournamentList(l *lua.LState) int { return 0 } - var cursor *tournamentListCursor + var cursor *TournamentListCursor cursorStr := l.OptString(6, "") if cursorStr != "" { cb, err := base64.StdEncoding.DecodeString(cursorStr) @@ -4729,14 +4729,14 @@ func (n *RuntimeLuaNakamaModule) tournamentList(l *lua.LState) int { l.ArgError(6, "expects cursor to be valid when provided") return 0 } - cursor = &tournamentListCursor{} + cursor = &TournamentListCursor{} if err := gob.NewDecoder(bytes.NewReader(cb)).Decode(cursor); err != nil { l.ArgError(6, "expects cursor to be valid when provided") return 0 } } - list, err := TournamentList(l.Context(), n.logger, n.db, categoryStart, categoryEnd, startTime, endTime, limit, cursor) + list, err := TournamentList(l.Context(), n.logger, n.db, n.leaderboardCache, categoryStart, categoryEnd, startTime, endTime, limit, cursor) if err != nil { l.RaiseError("error listing tournaments: %v", err.Error()) return 0 -- GitLab