Unverified Commit ba6ae4a4 authored by Simon Esposito's avatar Simon Esposito Committed by GitHub
Browse files

Fix tournament related bugs (#491) 

Fix tournament record update error when trying to set an invalid score.
Fixes to the tournament listing api and improvements to its filters.
Change the default limit when listing tournament records.
parent 911736d7

CHANGELOG.md

+4 −0
Original line number Diff line number Diff line
@@ -7,6 +7,10 @@ The format is based on [keep a changelog](http://keepachangelog.com) and this pr 
### Fixed

- Better handling of SSL connections in development configurations.

- Use correct error message and response code when RPC functions receive a request payload larger than allowed.

- Expose missing 'group_users_kick' function to the Lua runtime.

- Fix an issue that would cause an error when trying to update a tournament record with invalid data.

- Fix some issues around listing tournaments.

- Fix an issue that would prevent the insertion of a record in a tournament with no scheduled reset and end time.



## [2.14.1] - 2020-11-02

### Added

server/api_tournament.go

+12 −15
Original line number Diff line number Diff line
@@ -127,8 +127,8 @@ func (s *ApiServer) ListTournamentRecords(ctx context.Context, in *api.ListTourn 
			return nil, status.Error(codes.InvalidArgument, "Invalid limit - limit must be between 1 and 100.")

		}

		limit = in.GetLimit()

	} else if len(in.GetOwnerIds()) == 0 || in.GetCursor() != "" {

		limit = &wrappers.Int32Value{Value: 1}

	} else if len(in.GetOwnerIds()) == 0 || in.GetCursor() == "" {

		limit = &wrappers.Int32Value{Value: 10}

	}



	overrideExpiry := int64(0)
@@ -213,20 +213,21 @@ func (s *ApiServer) ListTournaments(ctx context.Context, in *api.ListTournaments 
		}

	}



	// If startTime and endTime are both not set the API will only return active or future tournaments.

	startTime := -1 // don't include start time in query

	if in.GetStartTime() != nil {

		startTime = int(in.GetStartTime().GetValue())

	}



	endTime := int(time.Now().UTC().AddDate(1, 0, 0).Unix()) // one year from now

	endTime := -1 // don't include end time in query

	if in.GetEndTime() != nil {

		endTime = int(in.GetEndTime().GetValue())

		if endTime < startTime {

		if endTime != 0 && endTime < startTime { // Allow 0 value to explicitly request tournaments with no end time set.

			return nil, status.Error(codes.InvalidArgument, "Tournament end time must be greater than start time.")

		}

	}



	limit := 1

	limit := 100

	if in.GetLimit() != nil {

		limit = int(in.GetLimit().GetValue())

		if limit < 1 || limit > 100 {
@@ -280,11 +281,7 @@ func (s *ApiServer) WriteTournamentRecord(ctx context.Context, in *api.WriteTour 
	}



	if in.GetTournamentId() == "" {

		return nil, status.Error(codes.InvalidArgument, "Tournament ID must be provided")

	}



	if in.GetTournamentId() == "" {

		return nil, status.Error(codes.InvalidArgument, "Invalid tournament ID.")

		return nil, status.Error(codes.InvalidArgument, "Tournament ID must be provided.")

	} else if in.GetRecord() == nil {

		return nil, status.Error(codes.InvalidArgument, "Invalid input, record score value is required.")

	} else if in.GetRecord().GetMetadata() != "" {
@@ -305,13 +302,13 @@ func (s *ApiServer) WriteTournamentRecord(ctx context.Context, in *api.WriteTour 
	record, err := TournamentRecordWrite(ctx, s.logger, s.db, s.leaderboardCache, s.leaderboardRankCache, in.GetTournamentId(), userID, username, in.GetRecord().GetScore(), in.GetRecord().GetSubscore(), in.GetRecord().GetMetadata())

	if err != nil {

		if err == ErrTournamentMaxSizeReached {

			return nil, status.Error(codes.InvalidArgument, "Tournament has reached max size.")

			return nil, status.Error(codes.FailedPrecondition, "Tournament has reached max size.")

		} else if err == ErrTournamentWriteMaxNumScoreReached {

			return nil, status.Error(codes.InvalidArgument, "Reached allowed max number of score attempts.")

			return nil, status.Error(codes.FailedPrecondition, "Reached allowed max number of score attempts.")

		} else if err == ErrTournamentWriteJoinRequired {

			return nil, status.Error(codes.InvalidArgument, "Must join tournament before attempting to write value.")

			return nil, status.Error(codes.FailedPrecondition, "Must join tournament before attempting to write value.")

		} else if err == ErrTournamentOutsideDuration {

			return nil, status.Error(codes.InvalidArgument, "Tournament is not active and cannot accept new scores.")

			return nil, status.Error(codes.FailedPrecondition, "Tournament is not active and cannot accept new scores.")

		} else {

			return nil, status.Error(codes.Internal, "Error writing score to tournament.")

		}
@@ -358,7 +355,7 @@ func (s *ApiServer) ListTournamentRecordsAroundOwner(ctx context.Context, in *ap 
		return nil, status.Error(codes.InvalidArgument, "Invalid tournament ID.")

	}



	limit := 1

	limit := 100

	if in.GetLimit() != nil {

		if in.GetLimit().Value < 1 || in.GetLimit().Value > 100 {

			return nil, status.Error(codes.InvalidArgument, "Invalid limit - limit must be between 1 and 100.")

server/core_tournament.go

+31 −21
Original line number Diff line number Diff line
@@ -396,11 +396,11 @@ func TournamentRecordWrite(ctx context.Context, logger *zap.Logger, db *sql.DB, 
	default:

		if leaderboard.SortOrder == LeaderboardSortOrderAscending {

			// Lower score is better.

			opSQL = "score = div((leaderboard_record.score + $5 - abs(leaderboard_record.score - $5)), 2), subscore = div((leaderboard_record.subscore + $6 - abs(leaderboard_record.subscore - $6)), 2)"

			opSQL = "score = div((leaderboard_record.score + $5 - abs(leaderboard_record.score - $5)), 2), subscore = div((leaderboard_record.subscore + $6 - abs(leaderboard_record.subscore - $6)), 2)" // (sub)score = min(db_value, $var)

			filterSQL = " WHERE (leaderboard_record.score > $5 OR leaderboard_record.subscore > $6)"

		} else {

			// Higher score is better.

			opSQL = "score = div((leaderboard_record.score + $5 + abs(leaderboard_record.score - $5)), 2), subscore = div((leaderboard_record.subscore + $6 + abs(leaderboard_record.subscore - $6)), 2)"

			opSQL = "score = div((leaderboard_record.score + $5 + abs(leaderboard_record.score - $5)), 2), subscore = div((leaderboard_record.subscore + $6 + abs(leaderboard_record.subscore - $6)), 2)" // (sub)score = max(db_value, $var)

			filterSQL = " WHERE (leaderboard_record.score < $5 OR leaderboard_record.subscore < $6)"

		}

		scoreDelta = score
@@ -453,13 +453,9 @@ func TournamentRecordWrite(ctx context.Context, logger *zap.Logger, db *sql.DB, 
		query := `INSERT INTO leaderboard_record (leaderboard_id, owner_id, username, score, subscore, metadata, expiry_time, max_num_score)

            VALUES ($1, $2, $3, $8, $9, COALESCE($7, '{}'::JSONB), $4, $10)

            ON CONFLICT (owner_id, leaderboard_id, expiry_time)

            DO UPDATE SET ` + opSQL + `, num_score = leaderboard_record.num_score + 1, metadata = COALESCE($7, leaderboard_record.metadata), username = COALESCE($3, leaderboard_record.username), update_time = now() ` + filterSQL +

			`RETURNING num_score, max_num_score`

            DO UPDATE SET ` + opSQL + `, num_score = leaderboard_record.num_score + 1, metadata = COALESCE($7, leaderboard_record.metadata), username = COALESCE($3, leaderboard_record.username), update_time = now()` + filterSQL

		params = append(params, scoreAbs, subscoreAbs, leaderboard.MaxNumScore)



		var dbNumScore int

		var dbMaxNumScore int



		tx, err := db.BeginTx(ctx, nil)

		if err != nil {

			logger.Error("Could not begin database transaction.", zap.Error(err))
@@ -467,7 +463,19 @@ func TournamentRecordWrite(ctx context.Context, logger *zap.Logger, db *sql.DB, 
		}



		if err := ExecuteInTx(ctx, tx, func() error {

			if err := tx.QueryRowContext(ctx, query, params...).Scan(&dbNumScore, &dbMaxNumScore); err != nil {

			recordQueryResult, err := tx.ExecContext(ctx, query, params...)

			if err != nil {

				return err

			}



			// A record was inserted or updated

			if rowsAffected, _ := recordQueryResult.RowsAffected(); rowsAffected > 0 {

				var dbNumScore int

				var dbMaxNumScore int



				err := tx.QueryRowContext(ctx, "SELECT num_score, max_num_score FROM leaderboard_record WHERE leaderboard_id = $1 AND owner_id = $2 AND expiry_time = $3", leaderboard.Id, ownerId, expiryTime).Scan(&dbNumScore, &dbMaxNumScore)

				if err != nil {

					logger.Error("Error reading leaderboard record.", zap.Error(err))

					return err

				}
@@ -488,6 +496,7 @@ func TournamentRecordWrite(ctx context.Context, logger *zap.Logger, db *sql.DB, 
						return ErrTournamentMaxSizeReached

					}

				}

			}



			return nil

		}); err != nil {
@@ -496,6 +505,7 @@ func TournamentRecordWrite(ctx context.Context, logger *zap.Logger, db *sql.DB, 
			} else {

				logger.Error("Could not write tournament record", zap.Error(err), zap.String("tournament_id", tournamentId), zap.String("owner_id", ownerId.String()))

			}



			return nil, err

		}

	}
@@ -623,7 +633,7 @@ func calculateTournamentDeadlines(startTime, endTime, duration int64, resetSched 
		endActiveUnix = startTime + duration

	}

	expiryUnix := endTime

	if endActiveUnix > expiryUnix {

	if expiryUnix > 0 && endActiveUnix > expiryUnix {

		// Cap the end active to the same time as the expiry.

		endActiveUnix = expiryUnix

	}
@@ -661,7 +671,7 @@ func parseTournament(scannable Scannable, now time.Time) (*api.Tournament, error 


	startActive, endActiveUnix, expiryUnix := calculateTournamentDeadlines(dbStartTime.Time.UTC().Unix(), endTime, int64(dbDuration), resetSchedule, now)



	if startActive > now.Unix() || endActiveUnix < now.Unix() {

	if startActive > now.Unix() || (endActiveUnix != 0 && endActiveUnix < now.Unix()) {

		canEnter = false

	}

server/leaderboard_cache.go

+4 −3
Original line number Diff line number Diff line
@@ -575,11 +575,12 @@ func (l *LocalLeaderboardCache) ListTournaments(now int64, categoryStart, catego 
			// 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 {

		if (endTime == 0 && leaderboard.EndTime != 0) || (endTime == -1 && (leaderboard.EndTime != 0 && leaderboard.EndTime < now)) || (endTime > 0 && (leaderboard.EndTime == 0 || leaderboard.EndTime > endTime)) {

			// if (endTime == 0 && leaderboard.EndTime != 0) || (endTime == -1 && 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.

			// - If end time filter is default (show only ongoing/future tournaments) and tournament has ended.

			// - If end time filter is set and tournament end time is below it.

			continue

		}

server/runtime_lua_nakama.go

+1 −0
Original line number Diff line number Diff line
@@ -239,6 +239,7 @@ func (n *RuntimeLuaNakamaModule) Loader(l *lua.LState) int { 
		"group_update":                       n.groupUpdate,

		"group_delete":                       n.groupDelete,

		"group_users_list":                   n.groupUsersList,

		"group_users_kick":                   n.groupUsersKick,

		"user_groups_list":                   n.userGroupsList,

		"friends_list":                       n.friendsList,

	}