Merge pull request #458 from danthe1st/uniform-qotw-leaderboard

Author: jasonlessenich

src/main/java/net/discordjug/javabot/api/routes/leaderboard/qotw/QOTWLeaderboardController.java

@@ -6,9 +6,11 @@
 import net.discordjug.javabot.api.routes.CaffeineCache;
 import net.discordjug.javabot.api.routes.leaderboard.qotw.model.QOTWUserData;
 import net.discordjug.javabot.systems.qotw.QOTWPointsService;
+import net.discordjug.javabot.systems.qotw.model.QOTWAccount;
 import net.discordjug.javabot.util.Pair;
 import net.dv8tion.jda.api.JDA;
 import net.dv8tion.jda.api.entities.Guild;
+import net.dv8tion.jda.api.entities.User;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
@@ -26,7 +28,7 @@
  */
 @RestController
 public class QOTWLeaderboardController extends CaffeineCache<Pair<Long, Integer>, List<QOTWUserData>> {
-	private static final int PAGE_AMOUNT = 8;
+	private static final int PAGE_AMOUNT = 10;
 	private final JDA jda;
 	private final QOTWPointsService pointsService;
 
@@ -65,11 +67,23 @@ public ResponseEntity<List<QOTWUserData>> getQOTWLeaderboard(
 		}
 		List<QOTWUserData> members = getCache().getIfPresent(new Pair<>(guild.getIdLong(), page));
 		if (members == null || members.isEmpty()) {
-			members = pointsService.getTopAccounts(PAGE_AMOUNT, page).stream()
-					.map(p -> QOTWUserData.of(p, jda.retrieveUserById(p.getUserId()).complete()))
+			List<QOTWAccount> topAccounts = pointsService.getTopAccounts(PAGE_AMOUNT, page);
+			members = topAccounts.stream()
+					.map(account -> new Pair<>(account, jda.retrieveUserById(account.getUserId()).complete()))
+					.filter(pair -> guild.isMember(pair.second()))
+					.map(pair -> createAPIAccount(pair.first(), pair.second(), topAccounts, page))
 					.toList();
 			getCache().put(new Pair<>(guild.getIdLong(), page), members);
 		}
 		return new ResponseEntity<>(members, HttpStatus.OK);
 	}
+
+	private QOTWUserData createAPIAccount(QOTWAccount account, User user, List<QOTWAccount> topAccounts, int page) {
+		return QOTWUserData.of(
+				account,
+				user,
+				//this can be inaccurate for later pages with multiple users having the same score on the previous page
+				//specifically, it counts all users on previous pages as strictly higher in the leaderboard
+				pointsService.getQOTWRank(account.getUserId(), topAccounts)+(page-1)*PAGE_AMOUNT);
+	}
 }

src/main/java/net/discordjug/javabot/api/routes/leaderboard/qotw/model/QOTWUserData.java

@@ -17,22 +17,25 @@
 @EqualsAndHashCode(callSuper = false)
 public class QOTWUserData extends UserData {
 	private QOTWAccount account;
+	private int rank;
 
 	/**
 	 * Creates a new {@link QOTWUserData} instance.
 	 *
 	 * @param account The {@link QOTWAccount} to use.
 	 * @param user A nullable {@link User}.
+	 * @param rank The position of the user in the QOTW leaderboard
 	 * @return The {@link QOTWUserData}.
 	 */
-	public static @NotNull QOTWUserData of(@NotNull QOTWAccount account, @Nullable User user) {
+	public static @NotNull QOTWUserData of(@NotNull QOTWAccount account, @Nullable User user, int rank) {
 		QOTWUserData data = new QOTWUserData();
 		data.setUserId(account.getUserId());
 		if (user != null) {
 			data.setUserName(user.getName());
 			data.setDiscriminator(user.getDiscriminator());
 			data.setEffectiveAvatarUrl(user.getEffectiveAvatarUrl());
 		}
+		data.setRank(rank);
 		data.setAccount(account);
 		return data;
 	}

src/main/java/net/discordjug/javabot/systems/qotw/QOTWPointsService.java

@@ -55,16 +55,36 @@ public QOTWAccount getOrCreateAccount(long userId) throws DataAccessException {
 	public int getQOTWRank(long userId) {
 		try{
 			List<QOTWAccount> accounts = pointsRepository.sortByPoints(getCurrentMonth());
-			return accounts.stream()
-					.map(QOTWAccount::getUserId)
-					.toList()
-					.indexOf(userId) + 1;
+			int currentRank = getQOTWRank(userId, accounts);
+			return currentRank;
 		} catch (DataAccessException e) {
 			ExceptionLogger.capture(e, getClass().getSimpleName());
 			return -1;
 		}
 	}
 
+	/**
+	 * Gets the given user's QOTW-Rank given a {@link List} of QOTW accounts.
+	 * @param userId The user whose rank should be returned.
+	 * @param accounts A list of QOTW accounts in descending order with respect to the score. This should at least contain the user (if they are ranked) and all users before them.
+	 * @return The QOTW-Rank as an integer.
+	 */
+	public int getQOTWRank(long userId, List<QOTWAccount> accounts) {
+		long lastScore = -1;
+		int currentRank = 1;
+		for (int i = 0; i < accounts.size(); i++) {
+			QOTWAccount account = accounts.get(i);
+			if (account.getPoints() != lastScore) {
+				currentRank = i + 1;
+				lastScore = account.getPoints();
+			}
+			if (account.getUserId() == userId) {
+				return currentRank;
+			}
+		}
+		return -1;
+	}
+
 	/**
 	 * Gets the given user's QOTW-Points.
 	 *
@@ -89,7 +109,7 @@ public long getPoints(long userId) {
 	 */
 	public List<Pair<QOTWAccount, Member>> getTopMembers(int n, Guild guild) {
 		try {
-			List<QOTWAccount> accounts = pointsRepository.sortByPoints(getCurrentMonth());
+			List<QOTWAccount> accounts = pointsRepository.getTopAccounts(getCurrentMonth(),1,(int)Math.ceil(n*1.5));
 			return accounts.stream()
 					.map(s -> new Pair<>(s, guild.getMemberById(s.getUserId())))
 					.filter(p->p.first().getPoints() > 0)
@@ -106,7 +126,7 @@ public List<Pair<QOTWAccount, Member>> getTopMembers(int n, Guild guild) {
 	 * Gets the specified amount of {@link QOTWAccount}s, sorted by their points.
 	 *
 	 * @param amount The amount to retrieve.
-	 * @param page The page to get.
+	 * @param page The page to get, starting at 1.
 	 * @return An unmodifiable {@link List} of {@link QOTWAccount}s.
 	 */
 	public List<QOTWAccount> getTopAccounts(int amount, int page) {

src/main/java/net/discordjug/javabot/systems/qotw/dao/QuestionPointsRepository.java

@@ -92,13 +92,13 @@ public List<QOTWAccount> sortByPoints(LocalDate startDate) throws DataAccessExce
 	 * Gets a specified amount of {@link QOTWAccount}s.
 	 *
 	 * @param startDate the minimum date points are considered
-	 * @param page The page.
+	 * @param page The page, starting at 1.
 	 * @param size The amount of {@link QOTWAccount}s to return.
 	 * @return A {@link List} containing the specified amount of {@link QOTWAccount}s.
 	 * @throws DataAccessException If an error occurs.
 	 */
 	public List<QOTWAccount> getTopAccounts(LocalDate startDate, int page, int size) throws DataAccessException {
-		return jdbcTemplate.query("SELECT user_id, SUM(points) FROM qotw_points WHERE obtained_at >= ? AND points > 0  GROUP BY user_id ORDER BY SUM(points) DESC LIMIT ? OFFSET ?",
+		return jdbcTemplate.query("SELECT user_id, SUM(points) FROM qotw_points WHERE obtained_at >= ? AND points > 0  GROUP BY user_id ORDER BY SUM(points) DESC, user_id ASC LIMIT ? OFFSET ?",
 				(rs,row)->this.read(rs),
 				startDate, size, Math.max(0, (page * size) - size));
 	}

src/main/java/net/discordjug/javabot/systems/qotw/jobs/QOTWChampionJob.java

@@ -35,7 +35,7 @@ public void execute() {
 			LocalDate month=LocalDate.now().minusMonths(1);
 			Role qotwChampionRole = botConfig.get(guild).getQotwConfig().getQOTWChampionRole();
 			if (qotwChampionRole != null) {
-				pointsRepository.getTopAccounts(month, 0, 1)
+				pointsRepository.getTopAccounts(month, 1, 1)
 				.stream()
 				.findFirst()
 				.ifPresent(best -> {

src/main/java/net/discordjug/javabot/systems/user_commands/leaderboard/QOTWLeaderboardSubcommand.java

@@ -11,13 +11,11 @@
 
 import xyz.dynxsty.dih4jda.interactions.commands.application.SlashCommand;
 import net.discordjug.javabot.systems.qotw.QOTWPointsService;
-import net.discordjug.javabot.systems.qotw.dao.QuestionPointsRepository;
 import net.discordjug.javabot.systems.qotw.model.QOTWAccount;
 import net.discordjug.javabot.util.ExceptionLogger;
 import net.discordjug.javabot.util.Pair;
 import net.discordjug.javabot.util.UserUtils;
 import net.dv8tion.jda.api.EmbedBuilder;
-import net.dv8tion.jda.api.entities.Guild;
 import net.dv8tion.jda.api.entities.Member;
 import net.dv8tion.jda.api.entities.Message;
 import net.dv8tion.jda.api.entities.MessageEmbed;
@@ -35,29 +33,27 @@ public class QOTWLeaderboardSubcommand extends SlashCommand.Subcommand {
 
 	private final QOTWPointsService pointsService;
 	private final ExecutorService asyncPool;
-	private final QuestionPointsRepository qotwPointsRepository;
 
 	/**
 	 * The constructor of this class, which sets the corresponding {@link SubcommandData}.
 	 * @param pointsService The {@link QOTWPointsService} managing {@link QOTWAccount}s
 	 * @param asyncPool The thread pool for asynchronous operations
-	 * @param qotwPointsRepository Dao object that represents the QOTW_POINTS SQL Table.
 	 */
-	public QOTWLeaderboardSubcommand(QOTWPointsService pointsService, ExecutorService asyncPool, QuestionPointsRepository qotwPointsRepository) {
+	public QOTWLeaderboardSubcommand(QOTWPointsService pointsService, ExecutorService asyncPool) {
 		setCommandData(new SubcommandData("qotw", "The QOTW Points Leaderboard."));
 		this.pointsService=pointsService;
 		this.asyncPool = asyncPool;
-		this.qotwPointsRepository = qotwPointsRepository;
 	}
 
 	@Override
 	public void execute(SlashCommandInteractionEvent event) {
 		event.deferReply().queue();
 		asyncPool.submit(() -> {
 			try {
-				WebhookMessageCreateAction<Message> action = event.getHook().sendMessageEmbeds(buildLeaderboardRankEmbed(event.getMember()));
+				List<Pair<QOTWAccount,Member>> topMembers = pointsService.getTopMembers(DISPLAY_COUNT, event.getGuild());
+				WebhookMessageCreateAction<Message> action = event.getHook().sendMessageEmbeds(buildLeaderboardRankEmbed(event.getMember(), topMembers));
 				// check whether the image may already been cached
-				byte[] array = LeaderboardCreator.attemptLoadFromCache(getCacheName(), ()->generateLeaderboard(event.getGuild()));
+				byte[] array = LeaderboardCreator.attemptLoadFromCache(getCacheName(topMembers), ()->generateLeaderboard(topMembers));
 				action.addFiles(FileUpload.fromData(new ByteArrayInputStream(array), Instant.now().getEpochSecond() + ".png")).queue();
 			} catch (IOException e) {
 				ExceptionLogger.capture(e, getClass().getSimpleName());
@@ -68,11 +64,12 @@ public void execute(SlashCommandInteractionEvent event) {
 	/**
 	 * Builds the Leaderboard Rank {@link MessageEmbed}.
 	 *
+	 * @param topMembers the accounts with the top QOTW users
 	 * @param member  The member which executed the command.
 	 * @return A {@link MessageEmbed} object.
 	 */
-	private MessageEmbed buildLeaderboardRankEmbed(Member member) {
-		int rank = pointsService.getQOTWRank(member.getIdLong());
+	private MessageEmbed buildLeaderboardRankEmbed(Member member, List<Pair<QOTWAccount, Member>> topMembers) {
+		int rank = findRankOfMember(member, topMembers);
 		String rankSuffix = switch (rank % 10) {
 			case 1 -> "st";
 			case 2 -> "nd";
@@ -90,49 +87,53 @@ private MessageEmbed buildLeaderboardRankEmbed(Member member) {
 				.build();
 	}
 
+	private int findRankOfMember(Member member, List<Pair<QOTWAccount, Member>> topMembers) {
+		return pointsService.getQOTWRank(member.getIdLong(), 
+				topMembers
+				.stream()
+				.map(Pair::first)
+				.toList());
+	}
+
 	/**
 	 * Draws a single "user card".
 	 *
 	 * @param leaderboardCreator handling actual drawing.
 	 * @param member  The member.
 	 * @param service The {@link QOTWPointsService}.
+	 * @param topMembers the accounts with the top QOTW users
 	 * @throws IOException If an error occurs.
 	 */
-	private void drawUserCard(LeaderboardCreator leaderboardCreator, @NotNull Member member, QOTWPointsService service) throws IOException {
-		leaderboardCreator.drawLeaderboardEntry(member, UserUtils.getUserTag(member.getUser()), service.getPoints(member.getIdLong()), service.getQOTWRank(member.getIdLong()));
+	private void drawUserCard(LeaderboardCreator leaderboardCreator, @NotNull Member member, QOTWPointsService service, List<Pair<QOTWAccount, Member>> topMembers) throws IOException {
+		leaderboardCreator.drawLeaderboardEntry(member, UserUtils.getUserTag(member.getUser()), service.getPoints(member.getIdLong()), findRankOfMember(member, topMembers));
 	}
 
 	/**
 	 * Draws and constructs the leaderboard image.
 	 *
-	 * @param guild   The current guild.
+	 * @param topMembers the accounts with the top QOTW users
 	 * @return The finished image as a {@link ByteArrayInputStream}.
 	 * @throws IOException If an error occurs.
 	 */
-	private @NotNull byte[] generateLeaderboard(Guild guild) throws IOException {
-		List<Pair<QOTWAccount, Member>> topMembers = pointsService.getTopMembers(DISPLAY_COUNT, guild);
-
+	private @NotNull byte[] generateLeaderboard(List<Pair<QOTWAccount, Member>> topMembers) throws IOException {
 		try(LeaderboardCreator creator = new LeaderboardCreator(Math.min(DISPLAY_COUNT, topMembers.size()), "QuestionOfTheWeekHeader")){
 			for (Pair<QOTWAccount, Member> pair : topMembers) {
-				drawUserCard(creator, pair.second(), pointsService);
+				drawUserCard(creator, pair.second(), pointsService, topMembers);
 			}
-			return creator.getImageBytes(getCacheName(), "qotw_leaderboard");
+			return creator.getImageBytes(getCacheName(topMembers), "qotw_leaderboard");
 		}
 	}
 
 	/**
 	 * Builds the cached image's name.
 	 *
+	 * @param topMembers the accounts with the top QOTW users
 	 * @return The image's cache name.
 	 */
-	private @NotNull String getCacheName() {
+	private @NotNull String getCacheName(List<Pair<QOTWAccount, Member>> topMembers) {
 		try {
-			List<QOTWAccount> accounts = qotwPointsRepository.sortByPoints(QOTWPointsService.getCurrentMonth())
-					.stream()
-					.limit(DISPLAY_COUNT)
-					.toList();
 			StringBuilder sb = new StringBuilder("qotw_leaderboard_");
-			accounts.forEach(account -> sb.append(String.format(":%s:%s", account.getUserId(), account.getPoints())));
+			topMembers.forEach(account -> sb.append(String.format(":%s:%s", account.first().getUserId(), account.first().getPoints())));
 			return sb.toString();
 		} catch (DataAccessException e) {
 			ExceptionLogger.capture(e, getClass().getSimpleName());

src/test/java/net/discordjug/javabot/systems/qotw/QOTWPointsServiceTest.java

@@ -0,0 +1,61 @@
+package net.discordjug.javabot.systems.qotw;
+
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import net.discordjug.javabot.systems.qotw.model.QOTWAccount;
+
+class QOTWPointsServiceTest {
+	
+	private QOTWPointsService pointsService = new QOTWPointsService(null);
+
+	@Test
+	void testGetQOTWRankNotPresent() {
+		assertEquals(-1, pointsService.getQOTWRank(0, List.of()));
+		assertEquals(-1, pointsService.getQOTWRank(0, List.of(createAccount(1, 1))));
+	}
+	
+	@Test
+	void testNormalQOTWRank() {
+		assertEquals(1, pointsService.getQOTWRank(1, List.of(createAccount(1, 1))));
+		assertEquals(2, pointsService.getQOTWRank(2, List.of(
+				createAccount(1, 2),
+				createAccount(2, 1)
+		)));
+	}
+	
+	@Test
+	void testQOTWRankWithTiesBefore() {
+		assertEquals(3, pointsService.getQOTWRank(1, List.of(
+				createAccount(2, 2),
+				createAccount(3, 2),
+				createAccount(1, 1)
+		)));
+	}
+	
+	@Test
+	void testQOTWRankWithTiesAtSamePosition() {
+		assertEquals(2, pointsService.getQOTWRank(1, List.of(
+				createAccount(2, 2),
+				createAccount(3, 1),
+				createAccount(1, 1)
+		)));
+		assertEquals(2, pointsService.getQOTWRank(1, List.of(
+				createAccount(2, 2),
+				createAccount(1, 1),
+				createAccount(3, 1)
+		)));
+	}
+
+	private QOTWAccount createAccount(long userId, int score) {
+		QOTWAccount account = new QOTWAccount();
+		account.setUserId(userId);
+		account.setPoints(score);
+		return account;
+	}
+
+}