From 3d2f40b5b187fe96121946e0acb051343403aae7 Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 29 Nov 2024 16:27:46 +0100 Subject: [PATCH 01/11] Modularization of task scheduling --- .../leaderboard/LeaderboardTaskScheduler.java | 77 +++++++ .../leaderboard/SlackMessageService.java | 212 ++---------------- .../tasks/SlackWeeklyLeaderboardTask.java | 172 ++++++++++++++ 3 files changed, 265 insertions(+), 196 deletions(-) create mode 100644 server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java create mode 100644 server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/SlackWeeklyLeaderboardTask.java diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java new file mode 100644 index 00000000..76d9a31a --- /dev/null +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java @@ -0,0 +1,77 @@ +package de.tum.in.www1.hephaestus.leaderboard; + +import de.tum.in.www1.hephaestus.leaderboard.tasks.SlackWeeklyLeaderboardTask; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.context.event.ApplicationReadyEvent; +import org.springframework.context.event.EventListener; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; +import org.springframework.scheduling.annotation.EnableScheduling; +import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; +import org.springframework.scheduling.support.CronExpression; +import org.springframework.scheduling.support.CronTrigger; +import org.springframework.stereotype.Service; + +@Order(value = Ordered.LOWEST_PRECEDENCE) +@EnableScheduling +@Service +public class LeaderboardTaskScheduler { + + private static final Logger logger = LoggerFactory.getLogger(LeaderboardTaskScheduler.class); + + @Value("${hephaestus.leaderboard.notification.enabled}") + private boolean runScheduledMessage; + + @Value("${hephaestus.leaderboard.schedule.day}") + private String scheduledDay; + + @Value("${hephaestus.leaderboard.schedule.time}") + private String scheduledTime; + + @Autowired + private ThreadPoolTaskScheduler taskScheduler; + + @Autowired + private SlackWeeklyLeaderboardTask slackWeeklyLeaderboardTask; + + + @EventListener(ApplicationReadyEvent.class) + public void activateTaskScheduler() { + + var timeParts = scheduledTime.split(":"); + + // CRON for the end of every leaderboard cycle + String cron = String.format( + "0 %s %s ? * %s", + timeParts.length > 1 ? timeParts[1] : 0, + timeParts[0], + scheduledDay + ); + + if (!CronExpression.isValidExpression(cron)) { + logger.error("Invalid cron expression: " + cron); + return; + } + + scheduleSlackMessage(cron); + + } + + /** + * Schedule a Slack message to be sent at the end of every leaderboard cycle. + */ + private void scheduleSlackMessage(String cron) { + if (!runScheduledMessage) return; + + if (!slackWeeklyLeaderboardTask.testSlackConnection()) { + logger.error("Failed to schedule Slack message"); + return; + } + + logger.info("Scheduling Slack message to run with {}", cron); + taskScheduler.schedule(slackWeeklyLeaderboardTask, new CronTrigger(cron)); + } +} diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/SlackMessageService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/SlackMessageService.java index 847a7a83..0ae6f8c5 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/SlackMessageService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/SlackMessageService.java @@ -1,8 +1,5 @@ package de.tum.in.www1.hephaestus.leaderboard; -import static com.slack.api.model.block.Blocks.*; -import static com.slack.api.model.block.composition.BlockCompositions.*; - import com.slack.api.bolt.App; import com.slack.api.methods.SlackApiException; import com.slack.api.methods.request.chat.ChatPostMessageRequest; @@ -11,59 +8,33 @@ import com.slack.api.model.User; import com.slack.api.model.block.LayoutBlock; import java.io.IOException; -import java.time.DayOfWeek; -import java.time.OffsetDateTime; -import java.time.temporal.TemporalAdjusters; import java.util.ArrayList; import java.util.List; -import java.util.Optional; -import java.util.function.Function; -import java.util.stream.IntStream; -import org.apache.commons.text.similarity.LevenshteinDistance; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.context.event.ApplicationReadyEvent; -import org.springframework.context.event.EventListener; -import org.springframework.core.Ordered; -import org.springframework.core.annotation.Order; -import org.springframework.scheduling.annotation.EnableScheduling; -import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; -import org.springframework.scheduling.support.CronExpression; -import org.springframework.scheduling.support.CronTrigger; import org.springframework.stereotype.Service; -@Order(value = Ordered.LOWEST_PRECEDENCE) -@EnableScheduling @Service public class SlackMessageService { private static final Logger logger = LoggerFactory.getLogger(SlackMessageService.class); - @Value("${hephaestus.leaderboard.notification.channel-id}") - private String channelId; - - @Value("${hephaestus.leaderboard.notification.enabled}") - private boolean runScheduledMessage; - - @Value("${hephaestus.host-url:localhost:8080}") - private String hephaestusUrl; - - @Value("${hephaestus.leaderboard.schedule.day}") - private String scheduledDay; - - @Value("${hephaestus.leaderboard.schedule.time}") - private String scheduledTime; - @Autowired private App slackApp; - @Autowired - private LeaderboardService leaderboardService; - - @Autowired - private ThreadPoolTaskScheduler taskScheduler; + /** + * Gets all members of the Slack workspace. + * @return + */ + public List getAllMembers() { + try { + return slackApp.client().usersList(r -> r).getMembers(); + } catch (IOException | SlackApiException e) { + logger.error("Failed to get all members from Slack: " + e.getMessage()); + return new ArrayList<>(); + } + } /** * Sends a message to the specified Slack channel. @@ -88,39 +59,10 @@ public void sendMessage(String channelId, List blocks, String fallb } } - @EventListener(ApplicationReadyEvent.class) - public void activateTaskScheduler() { - if (!runScheduledMessage) { - return; - } - - var timeParts = scheduledTime.split(":"); - - String cron = String.format( - "0 %s %s ? * %s", - timeParts.length > 1 ? timeParts[1] : 0, - timeParts[0], - scheduledDay - ); - - if (!CronExpression.isValidExpression(cron)) { - logger.error("Invalid cron expression: " + cron); - return; - } - - logger.info("Scheduling Slack message to run with {}", cron); - taskScheduler.schedule(new SlackWeeklyLeaderboardTask(), new CronTrigger(cron)); - } - /** - * Test the Slack app initialization on application startup. + * Test if the Slack app is correctly initialized. */ - @EventListener(ApplicationReadyEvent.class) - public void run() { - if (!runScheduledMessage) { - logger.info("Slack scheduled messages are disabled, skipping Slack app init test."); - return; - } + public boolean initTest() { logger.info("Testing Slack app initialization..."); AuthTestResponse response; try { @@ -132,132 +74,10 @@ public void run() { } if (response.isOk()) { logger.info("Slack app is successfully initialized."); + return true; } else { logger.error("Failed to initialize Slack app: " + response.getError()); - } - } - - /** - * Task to send a weekly leaderboard message to the Slack channel. - * @see SlackMessageService#activateTaskScheduler() - */ - private class SlackWeeklyLeaderboardTask implements Runnable { - - /** - * Gets the Slack handles of the top 3 reviewers in the given time frame. - * @return - */ - private List getTop3SlackReviewers(OffsetDateTime after, OffsetDateTime before) { - var leaderboard = leaderboardService.createLeaderboard(after, before, Optional.empty()); - var top3 = leaderboard.subList(0, Math.min(3, leaderboard.size())); - logger.debug("Top 3 Users of the last week: " + top3.stream().map(e -> e.user().name()).toList()); - - List allSlackUsers; - try { - allSlackUsers = slackApp.client().usersList(r -> r).getMembers(); - } catch (SlackApiException | IOException e) { - logger.error("Failed to get Slack users: " + e.getMessage()); - return new ArrayList<>(); - } - - return top3.stream().map(mapToSlackUser(allSlackUsers)).filter(user -> user != null).toList(); - } - - private Function mapToSlackUser(List allSlackUsers) { - return entry -> { - var exactUser = allSlackUsers - .stream() - .filter( - user -> - user.getName().equals(entry.user().name()) || - (user.getProfile().getEmail() != null && - user.getProfile().getEmail().equals(entry.user().email())) - ) - .findFirst(); - if (exactUser.isPresent()) { - return exactUser.get(); - } - - // find through String edit distance - return allSlackUsers - .stream() - .min((a, b) -> - Integer.compare( - LevenshteinDistance.getDefaultInstance().apply(entry.user().name(), a.getName()), - LevenshteinDistance.getDefaultInstance().apply(entry.user().name(), b.getName()) - ) - ) - .orElse(null); - }; - } - - private String formatDateForURL(OffsetDateTime date) { - return date.format(java.time.format.DateTimeFormatter.ISO_OFFSET_DATE_TIME).replace("+", "%2B"); - } - - @Override - public void run() { - // get date in unix format - long currentDate = OffsetDateTime.now().toEpochSecond(); - // Calculate the the last leaderboard schedule - String[] timeParts = scheduledTime.split(":"); - OffsetDateTime before = OffsetDateTime.now() - .with(TemporalAdjusters.previousOrSame(DayOfWeek.of(Integer.parseInt(scheduledDay)))) - .withHour(Integer.parseInt(timeParts[0])) - .withMinute(timeParts.length > 0 ? Integer.parseInt(timeParts[1]) : 0) - .withSecond(0) - .withNano(0); - OffsetDateTime after = before.minusWeeks(1); - - var top3reviewers = getTop3SlackReviewers(after, before); - - logger.info("Sending scheduled message to Slack channel..."); - - List blocks = asBlocks( - header(header -> - header.text(plainText(pt -> pt.text(":newspaper: Reviews of the last week :newspaper:"))) - ), - context(context -> - context.elements( - List.of( - markdownText( - " | " + hephaestusUrl - ) - ) - ) - ), - divider(), - section(section -> - section.text( - markdownText( - "Another *review leaderboard* has concluded. You can check out your placement <" + - hephaestusUrl + - "?after=" + - formatDateForURL(after) + - "&before=" + - formatDateForURL(before) + - "|here>." - ) - ) - ), - section(section -> section.text(markdownText("Special thanks to our top 3 reviewers of last week:"))), - section(section -> - section.text( - markdownText( - IntStream.range(0, top3reviewers.size()) - .mapToObj(i -> ((i + 1) + ". <@" + top3reviewers.get(i).getId() + ">")) - .reduce((a, b) -> a + "\n" + b) - .orElse("") - ) - ) - ), - section(section -> section.text(markdownText("Happy coding and reviewing! :rocket:"))) - ); - try { - sendMessage(channelId, blocks, "Reviews of the last week"); - } catch (IOException | SlackApiException e) { - logger.error("Failed to send scheduled message to Slack channel: " + e.getMessage()); - } + return false; } } } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/SlackWeeklyLeaderboardTask.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/SlackWeeklyLeaderboardTask.java new file mode 100644 index 00000000..9315f006 --- /dev/null +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/SlackWeeklyLeaderboardTask.java @@ -0,0 +1,172 @@ +package de.tum.in.www1.hephaestus.leaderboard.tasks; + +import static com.slack.api.model.block.Blocks.*; +import static com.slack.api.model.block.composition.BlockCompositions.*; + +import com.slack.api.methods.SlackApiException; +import com.slack.api.model.User; +import com.slack.api.model.block.LayoutBlock; +import de.tum.in.www1.hephaestus.leaderboard.LeaderboardEntryDTO; +import de.tum.in.www1.hephaestus.leaderboard.LeaderboardService; +import de.tum.in.www1.hephaestus.leaderboard.SlackMessageService; +import java.io.IOException; +import java.time.DayOfWeek; +import java.time.OffsetDateTime; +import java.time.temporal.TemporalAdjusters; +import java.util.List; +import java.util.Optional; +import java.util.function.Function; +import java.util.stream.IntStream; +import org.apache.commons.text.similarity.LevenshteinDistance; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Component; + +/** + * Task to send a weekly leaderboard message to the Slack channel. + * @see SlackMessageService#activateTaskScheduler() + */ +@Component +public class SlackWeeklyLeaderboardTask implements Runnable { + private static final Logger logger = LoggerFactory.getLogger(SlackWeeklyLeaderboardTask.class); + + @Value("${hephaestus.leaderboard.notification.channel-id}") + private String channelId; + + @Value("${hephaestus.leaderboard.notification.enabled}") + private boolean runScheduledMessage; + + @Value("${hephaestus.host-url:localhost:8080}") + private String hephaestusUrl; + + @Value("${hephaestus.leaderboard.schedule.day}") + private String scheduledDay; + + @Value("${hephaestus.leaderboard.schedule.time}") + private String scheduledTime; + + @Autowired + private SlackMessageService slackMessageService; + + @Autowired + private LeaderboardService leaderboardService; + + /** + * Test the Slack connection. + * @return + */ + public boolean testSlackConnection() { + return runScheduledMessage && slackMessageService.initTest(); + } + + /** + * Gets the Slack handles of the top 3 reviewers in the given time frame. + * @return + */ + private List getTop3SlackReviewers(OffsetDateTime after, OffsetDateTime before) { + var leaderboard = leaderboardService.createLeaderboard(after, before, Optional.empty()); + var top3 = leaderboard.subList(0, Math.min(3, leaderboard.size())); + logger.debug("Top 3 Users of the last week: " + top3.stream().map(e -> e.user().name()).toList()); + + List allSlackUsers = slackMessageService.getAllMembers(); + + return top3.stream().map(mapToSlackUser(allSlackUsers)).filter(user -> user != null).toList(); + } + + private Function mapToSlackUser(List allSlackUsers) { + return entry -> { + var exactUser = allSlackUsers + .stream() + .filter( + user -> + user.getName().equals(entry.user().name()) || + (user.getProfile().getEmail() != null && + user.getProfile().getEmail().equals(entry.user().email())) + ) + .findFirst(); + if (exactUser.isPresent()) { + return exactUser.get(); + } + + // find through String edit distance + return allSlackUsers + .stream() + .min((a, b) -> + Integer.compare( + LevenshteinDistance.getDefaultInstance().apply(entry.user().name(), a.getName()), + LevenshteinDistance.getDefaultInstance().apply(entry.user().name(), b.getName()) + ) + ) + .orElse(null); + }; + } + + private String formatDateForURL(OffsetDateTime date) { + return date.format(java.time.format.DateTimeFormatter.ISO_OFFSET_DATE_TIME).replace("+", "%2B"); + } + + @Override + public void run() { + // get date in unix format + long currentDate = OffsetDateTime.now().toEpochSecond(); + // Calculate the the last leaderboard schedule + String[] timeParts = scheduledTime.split(":"); + OffsetDateTime before = OffsetDateTime.now() + .with(TemporalAdjusters.previousOrSame(DayOfWeek.of(Integer.parseInt(scheduledDay)))) + .withHour(Integer.parseInt(timeParts[0])) + .withMinute(timeParts.length > 0 ? Integer.parseInt(timeParts[1]) : 0) + .withSecond(0) + .withNano(0); + OffsetDateTime after = before.minusWeeks(1); + + var top3reviewers = getTop3SlackReviewers(after, before); + + logger.info("Sending scheduled message to Slack channel..."); + + List blocks = asBlocks( + header(header -> header.text(plainText(pt -> pt.text(":newspaper: Reviews of the last week :newspaper:")))), + context(context -> + context.elements( + List.of( + markdownText( + " | " + hephaestusUrl + ) + ) + ) + ), + divider(), + section(section -> + section.text( + markdownText( + "Another *review leaderboard* has concluded. You can check out your placement <" + + hephaestusUrl + + "?after=" + + formatDateForURL(after) + + "&before=" + + formatDateForURL(before) + + "|here>." + ) + ) + ), + section(section -> section.text(markdownText("Special thanks to our top 3 reviewers of last week:"))), + section(section -> + section.text( + markdownText( + IntStream.range(0, top3reviewers.size()) + .mapToObj(i -> ((i + 1) + ". <@" + top3reviewers.get(i).getId() + ">")) + .reduce((a, b) -> a + "\n" + b) + .orElse("") + ) + ) + ), + section(section -> section.text(markdownText("Happy coding and reviewing! :rocket:"))) + ); + try { + slackMessageService.sendMessage(channelId, blocks, "Reviews of the last week"); + } catch (IOException | SlackApiException e) { + logger.error("Failed to send scheduled message to Slack channel: " + e.getMessage()); + } + } +} From dda9e42eb0787ff76ddbd3718a6ceb760a1742b7 Mon Sep 17 00:00:00 2001 From: GODrums Date: Fri, 29 Nov 2024 16:51:33 +0100 Subject: [PATCH 02/11] Improve inline documentation --- .../hephaestus/leaderboard/LeaderboardTaskScheduler.java | 4 ++++ .../www1/hephaestus/leaderboard/SlackMessageService.java | 7 ++++++- .../leaderboard/tasks/SlackWeeklyLeaderboardTask.java | 4 ++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java index 76d9a31a..4577da5c 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java @@ -15,6 +15,10 @@ import org.springframework.scheduling.support.CronTrigger; import org.springframework.stereotype.Service; +/** + * Schedules tasks to run at the end of every leaderboard cycle. + * @see SlackWeeklyLeaderboardTask + */ @Order(value = Ordered.LOWEST_PRECEDENCE) @EnableScheduling @Service diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/SlackMessageService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/SlackMessageService.java index 0ae6f8c5..48741894 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/SlackMessageService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/SlackMessageService.java @@ -15,6 +15,10 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; +/** + * Light wrapper around the Slack App to send messages to the Slack workspace. + * @implNote Use the exposed method to test the Slack connection beforehand. + */ @Service public class SlackMessageService { @@ -60,7 +64,8 @@ public void sendMessage(String channelId, List blocks, String fallb } /** - * Test if the Slack app is correctly initialized. + * Tests if the Slack app is correctly initialized and has access to the workspace. + * Does not guarantee that the app has the necessary permissions to send messages. */ public boolean initTest() { logger.info("Testing Slack app initialization..."); diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/SlackWeeklyLeaderboardTask.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/SlackWeeklyLeaderboardTask.java index 9315f006..0c30e2f5 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/SlackWeeklyLeaderboardTask.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/SlackWeeklyLeaderboardTask.java @@ -26,7 +26,7 @@ /** * Task to send a weekly leaderboard message to the Slack channel. - * @see SlackMessageService#activateTaskScheduler() + * @see SlackMessageService */ @Component public class SlackWeeklyLeaderboardTask implements Runnable { @@ -55,7 +55,7 @@ public class SlackWeeklyLeaderboardTask implements Runnable { /** * Test the Slack connection. - * @return + * @return {@code true} if the connection is valid, {@code false} otherwise. */ public boolean testSlackConnection() { return runScheduledMessage && slackMessageService.initTest(); From 07b0c5c314f67ad7f02373e0ea68c5b7a5848db1 Mon Sep 17 00:00:00 2001 From: GODrums Date: Sat, 30 Nov 2024 12:41:48 +0100 Subject: [PATCH 03/11] Add league points calculation and new task for leaderboard --- .../hephaestus/gitprovider/user/User.java | 3 + .../LeaguePointsCalculationService.java | 61 +++++++++++++++++ .../tasks/LeaguePointsUpdateTask.java | 68 +++++++++++++++++++ 3 files changed, 132 insertions(+) create mode 100644 server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java create mode 100644 server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/User.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/User.java index c5b1b5fe..2301347f 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/User.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/User.java @@ -98,6 +98,9 @@ public class User extends BaseGitServiceEntity { @ToString.Exclude private Set reviewComments = new HashSet<>(); + // Current ranking points for the leaderboard leagues + private int leaguePoints; + public enum Type { USER, ORGANIZATION, BOT } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java new file mode 100644 index 00000000..f20c61a4 --- /dev/null +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java @@ -0,0 +1,61 @@ +package de.tum.in.www1.hephaestus.leaderboard; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; + +@Service +public class LeaguePointsCalculationService { + private final Logger logger = LoggerFactory.getLogger(LeaguePointsCalculationService.class); + + public int calculateNewPoints(int oldPoints, LeaderboardEntryDTO entry) { + // Base decay - the higher the points, the more you lose + int decay = calculateDecay(oldPoints); + + // Activity bonus based on leaderboard score + int activityBonus = calculateActivityBonus(entry.score()); + + // Additional bonus for review diversity + int diversityBonus = calculateDiversityBonus( + entry.numberOfApprovals(), + entry.numberOfChangeRequests(), + entry.numberOfComments(), + entry.numberOfCodeComments() + ); + + // Calculate final point change + int pointChange = activityBonus + diversityBonus - decay; + + // Apply minimum change to prevent extreme swings + int newPoints = Math.max(0, oldPoints + pointChange); + + logger.info("Points calculation: old={}, decay={}, activity={}, diversity={}, new={}", + oldPoints, decay, activityBonus, diversityBonus, newPoints); + + return newPoints; + } + + private int calculateDecay(int currentPoints) { + // 5% decay of current points, minimum 10 points if they have any points + return currentPoints > 0 ? Math.max(10, (int)(currentPoints * 0.05)) : 0; + } + + private int calculateActivityBonus(int score) { + // Convert leaderboard score directly to points with diminishing returns + return (int)(Math.sqrt(score) * 10); + } + + private int calculateDiversityBonus(int approvals, int changes, int comments, int codeComments) { + // Reward diverse review activity + int totalInteractions = approvals + changes + comments; + if (totalInteractions == 0) return 0; + + // Calculate how evenly distributed the review types are + double distribution = (double)(Math.min(approvals, Math.min(changes, comments))) / Math.max(approvals, Math.max(changes, comments)); + + // Bonus for code comments + int codeCommentBonus = Math.min(50, codeComments * 2); + + return (int)(distribution * 30) + codeCommentBonus; + } +} diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java new file mode 100644 index 00000000..0f4e9c8c --- /dev/null +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java @@ -0,0 +1,68 @@ +package de.tum.in.www1.hephaestus.leaderboard.tasks; + +import de.tum.in.www1.hephaestus.gitprovider.user.UserRepository; +import de.tum.in.www1.hephaestus.leaderboard.LeaderboardEntryDTO; +import de.tum.in.www1.hephaestus.leaderboard.LeaderboardService; +import de.tum.in.www1.hephaestus.leaderboard.LeaguePointsCalculationService; +import java.time.DayOfWeek; +import java.time.OffsetDateTime; +import java.time.temporal.TemporalAdjusters; +import java.util.List; +import java.util.Optional; +import java.util.function.Consumer; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Component; + +@Component +public class LeaguePointsUpdateTask implements Runnable { + + @Value("${hephaestus.leaderboard.schedule.day}") + private String scheduledDay; + + @Value("${hephaestus.leaderboard.schedule.time}") + private String scheduledTime; + + @Autowired + private UserRepository userRepository; + @Autowired + private LeaderboardService leaderboardService; + @Autowired + private LeaguePointsCalculationService leaguePointsCalculationService; + + @Override + public void run() { + List leaderboard = getLatestLeaderboard(); + leaderboard.forEach(updateLeaderboardEntry()); + } + + /** + * Returns a consumer that updates the ranking points of a user based on its leaderboard entry. + * @return + */ + private Consumer updateLeaderboardEntry() { + return entry -> { + var user = userRepository.findByLogin(entry.user().login()).orElseThrow(); + int newPoints = leaguePointsCalculationService.calculateNewPoints(user.getLeaguePoints(), entry); + user.setLeaguePoints(newPoints); + userRepository.save(user); + }; + } + + /** + * Returns the most recently completed leaderboard + * @return + */ + private List getLatestLeaderboard() { + String[] timeParts = scheduledTime.split(":"); + OffsetDateTime before = OffsetDateTime.now() + .with(TemporalAdjusters.previousOrSame(DayOfWeek.of(Integer.parseInt(scheduledDay)))) + .withHour(Integer.parseInt(timeParts[0])) + .withMinute(timeParts.length > 0 ? Integer.parseInt(timeParts[1]) : 0) + .withSecond(0) + .withNano(0); + OffsetDateTime after = before.minusWeeks(1); + return leaderboardService.createLeaderboard(after, before, Optional.empty()); + } + +} From e75b2550faf5260c8580506938f5c119b626f44e Mon Sep 17 00:00:00 2001 From: GODrums Date: Sat, 30 Nov 2024 12:44:58 +0100 Subject: [PATCH 04/11] Liquibase changelog --- .../resources/db/changelog/1732966971482_changelog.xml | 10 ++++++++++ .../src/main/resources/db/master.xml | 1 + 2 files changed, 11 insertions(+) create mode 100644 server/application-server/src/main/resources/db/changelog/1732966971482_changelog.xml diff --git a/server/application-server/src/main/resources/db/changelog/1732966971482_changelog.xml b/server/application-server/src/main/resources/db/changelog/1732966971482_changelog.xml new file mode 100644 index 00000000..ba965f5e --- /dev/null +++ b/server/application-server/src/main/resources/db/changelog/1732966971482_changelog.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/server/application-server/src/main/resources/db/master.xml b/server/application-server/src/main/resources/db/master.xml index 23545da8..5e23a6ef 100644 --- a/server/application-server/src/main/resources/db/master.xml +++ b/server/application-server/src/main/resources/db/master.xml @@ -8,4 +8,5 @@ + From e07bc898f7ab3796a957034ddfdc2d655de20304 Mon Sep 17 00:00:00 2001 From: GODrums Date: Sun, 1 Dec 2024 23:33:32 +0100 Subject: [PATCH 05/11] Schedule point calculation --- .../gitprovider/user/UserRepository.java | 8 ++ .../leaderboard/LeaderboardTaskScheduler.java | 12 ++- .../LeaguePointsCalculationService.java | 98 ++++++++++++------- .../tasks/LeaguePointsUpdateTask.java | 6 +- .../db/changelog/1732966971482_changelog.xml | 2 +- 5 files changed, 87 insertions(+), 39 deletions(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/UserRepository.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/UserRepository.java index 72567beb..3da3833f 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/UserRepository.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/user/UserRepository.java @@ -17,6 +17,14 @@ public interface UserRepository extends JpaRepository { """) Optional findByLogin(@Param("login") String login); + @Query(""" + SELECT u + FROM User u + LEFT JOIN FETCH u.mergedPullRequests + WHERE u.login = :login + """) + Optional findByLoginWithEagerMergedPullRequests(@Param("login") String login); + @Query(""" SELECT u FROM User u diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java index 4577da5c..5e1c4ecb 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java @@ -1,5 +1,6 @@ package de.tum.in.www1.hephaestus.leaderboard; +import de.tum.in.www1.hephaestus.leaderboard.tasks.LeaguePointsUpdateTask; import de.tum.in.www1.hephaestus.leaderboard.tasks.SlackWeeklyLeaderboardTask; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,6 +41,8 @@ public class LeaderboardTaskScheduler { @Autowired private SlackWeeklyLeaderboardTask slackWeeklyLeaderboardTask; + @Autowired + private LeaguePointsUpdateTask leaguePointsUpdateTask; @EventListener(ApplicationReadyEvent.class) @@ -61,7 +64,7 @@ public void activateTaskScheduler() { } scheduleSlackMessage(cron); - + scheduleLeaguePointsUpdate(cron); } /** @@ -78,4 +81,11 @@ private void scheduleSlackMessage(String cron) { logger.info("Scheduling Slack message to run with {}", cron); taskScheduler.schedule(slackWeeklyLeaderboardTask, new CronTrigger(cron)); } + + private void scheduleLeaguePointsUpdate(String cron) { + // if (!runScheduledMessage) return; + + logger.info("Scheduling league points update to run with {}", cron); + taskScheduler.schedule(leaguePointsUpdateTask, new CronTrigger(cron)); + } } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java index f20c61a4..72af2518 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java @@ -1,5 +1,8 @@ package de.tum.in.www1.hephaestus.leaderboard; +import de.tum.in.www1.hephaestus.gitprovider.pullrequest.PullRequest; +import de.tum.in.www1.hephaestus.gitprovider.user.User; +import java.time.OffsetDateTime; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; @@ -8,54 +11,79 @@ public class LeaguePointsCalculationService { private final Logger logger = LoggerFactory.getLogger(LeaguePointsCalculationService.class); - public int calculateNewPoints(int oldPoints, LeaderboardEntryDTO entry) { - // Base decay - the higher the points, the more you lose - int decay = calculateDecay(oldPoints); - - // Activity bonus based on leaderboard score - int activityBonus = calculateActivityBonus(entry.score()); - - // Additional bonus for review diversity - int diversityBonus = calculateDiversityBonus( - entry.numberOfApprovals(), - entry.numberOfChangeRequests(), - entry.numberOfComments(), - entry.numberOfCodeComments() - ); - + public int calculateNewPoints(User user, LeaderboardEntryDTO entry) { + // Initialize new players with 1000 points + if (user.getLeaguePoints() == 0) { + user.setLeaguePoints(1000); + } + + int oldPoints = user.getLeaguePoints(); + + double kFactor = getKFactor(user); + // Base decay + int decay = calculateDecay(user.getLeaguePoints()); + // Bonus based on leaderboard score + int performanceBonus = calculatePerformanceBonus(entry.score()); + // Additional bonus for placements + int placementBonus = calculatePlacementBonus(entry.rank()); // Calculate final point change - int pointChange = activityBonus + diversityBonus - decay; - + int pointChange = (int) (kFactor * (performanceBonus - decay)); // Apply minimum change to prevent extreme swings int newPoints = Math.max(0, oldPoints + pointChange); - logger.info("Points calculation: old={}, decay={}, activity={}, diversity={}, new={}", - oldPoints, decay, activityBonus, diversityBonus, newPoints); - + logger.info("Points calculation: old={}, k={} decay={}, performanceBonus={}, placement={}, pointchange={}, new={}", + oldPoints, kFactor, decay, performanceBonus, placementBonus, pointChange, newPoints); + return newPoints; } + + /** + * Calculate the K factor for the user based on their current points and if they are a new player. + * @param user + * @return + * @see Wikipedia: Most accurate K-factor + */ + private double getKFactor(User user) { + if (isNewPlayer(user)) { + return 2.0; + } else if (user.getLeaguePoints() < 1400) { + return 1.5; + } else if (user.getLeaguePoints() < 1800) { + return 1.2; + } else { + return 1.1; + } + } + + /** + * Check if the user's earliest merged pull request is within the last 30 days. + * @param user + * @return + */ + private boolean isNewPlayer(User user) { + return user.getMergedPullRequests().stream() + .filter(PullRequest::isMerged) + .map(PullRequest::getMergedAt) + .anyMatch(date -> date.isAfter(OffsetDateTime.now().minusDays(30))); + } + /** + * Calculate the base decay in points based on the current points. + * @param currentPoints + * @return + */ private int calculateDecay(int currentPoints) { - // 5% decay of current points, minimum 10 points if they have any points + // 10% decay of current points, minimum 10 points if they have any points return currentPoints > 0 ? Math.max(10, (int)(currentPoints * 0.05)) : 0; } - private int calculateActivityBonus(int score) { + private int calculatePerformanceBonus(int score) { // Convert leaderboard score directly to points with diminishing returns return (int)(Math.sqrt(score) * 10); } - - private int calculateDiversityBonus(int approvals, int changes, int comments, int codeComments) { - // Reward diverse review activity - int totalInteractions = approvals + changes + comments; - if (totalInteractions == 0) return 0; - - // Calculate how evenly distributed the review types are - double distribution = (double)(Math.min(approvals, Math.min(changes, comments))) / Math.max(approvals, Math.max(changes, comments)); - - // Bonus for code comments - int codeCommentBonus = Math.min(50, codeComments * 2); - - return (int)(distribution * 30) + codeCommentBonus; + + private int calculatePlacementBonus(int placement) { + // Bonus for top 3 placements + return placement <= 3 ? 20 * (4 - placement) : 0; } } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java index 0f4e9c8c..41c3a69e 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java @@ -4,6 +4,7 @@ import de.tum.in.www1.hephaestus.leaderboard.LeaderboardEntryDTO; import de.tum.in.www1.hephaestus.leaderboard.LeaderboardService; import de.tum.in.www1.hephaestus.leaderboard.LeaguePointsCalculationService; +import jakarta.transaction.Transactional; import java.time.DayOfWeek; import java.time.OffsetDateTime; import java.time.temporal.TemporalAdjusters; @@ -31,6 +32,7 @@ public class LeaguePointsUpdateTask implements Runnable { private LeaguePointsCalculationService leaguePointsCalculationService; @Override + @Transactional public void run() { List leaderboard = getLatestLeaderboard(); leaderboard.forEach(updateLeaderboardEntry()); @@ -42,8 +44,8 @@ public void run() { */ private Consumer updateLeaderboardEntry() { return entry -> { - var user = userRepository.findByLogin(entry.user().login()).orElseThrow(); - int newPoints = leaguePointsCalculationService.calculateNewPoints(user.getLeaguePoints(), entry); + var user = userRepository.findByLoginWithEagerMergedPullRequests(entry.user().login()).orElseThrow(); + int newPoints = leaguePointsCalculationService.calculateNewPoints(user, entry); user.setLeaguePoints(newPoints); userRepository.save(user); }; diff --git a/server/application-server/src/main/resources/db/changelog/1732966971482_changelog.xml b/server/application-server/src/main/resources/db/changelog/1732966971482_changelog.xml index ba965f5e..215a8d73 100644 --- a/server/application-server/src/main/resources/db/changelog/1732966971482_changelog.xml +++ b/server/application-server/src/main/resources/db/changelog/1732966971482_changelog.xml @@ -2,7 +2,7 @@ - + From 4301a087b455668429692f5dc117941e25f0515b Mon Sep 17 00:00:00 2001 From: GODrums Date: Mon, 2 Dec 2024 00:20:32 +0100 Subject: [PATCH 06/11] Introduce PlacementBonus --- .../hephaestus/leaderboard/LeaguePointsCalculationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java index 72af2518..89b212b4 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java @@ -27,7 +27,7 @@ public int calculateNewPoints(User user, LeaderboardEntryDTO entry) { // Additional bonus for placements int placementBonus = calculatePlacementBonus(entry.rank()); // Calculate final point change - int pointChange = (int) (kFactor * (performanceBonus - decay)); + int pointChange = (int) (kFactor * (performanceBonus + placementBonus - decay)); // Apply minimum change to prevent extreme swings int newPoints = Math.max(0, oldPoints + pointChange); From 0e8c58a57259a8d2ea40560d3fed4c4e5fd13e82 Mon Sep 17 00:00:00 2001 From: GODrums Date: Mon, 2 Dec 2024 16:04:41 +0100 Subject: [PATCH 07/11] Improve JavaDocs --- .../hephaestus/leaderboard/LeaderboardTaskScheduler.java | 3 +-- .../leaderboard/LeaguePointsCalculationService.java | 2 +- .../leaderboard/tasks/LeaguePointsUpdateTask.java | 8 ++++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java index 00d7e0a9..f24e9495 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardTaskScheduler.java @@ -41,6 +41,7 @@ public class LeaderboardTaskScheduler { @Autowired private SlackWeeklyLeaderboardTask slackWeeklyLeaderboardTask; + @Autowired private LeaguePointsUpdateTask leaguePointsUpdateTask; @@ -83,8 +84,6 @@ private void scheduleSlackMessage(String cron) { } private void scheduleLeaguePointsUpdate(String cron) { - // if (!runScheduledMessage) return; - logger.info("Scheduling league points update to run with {}", cron); taskScheduler.schedule(leaguePointsUpdateTask, new CronTrigger(cron)); } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java index 89b212b4..eaa82605 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java @@ -73,7 +73,7 @@ private boolean isNewPlayer(User user) { * @return */ private int calculateDecay(int currentPoints) { - // 10% decay of current points, minimum 10 points if they have any points + // 5% decay of current points, minimum 10 points if they have any points return currentPoints > 0 ? Math.max(10, (int)(currentPoints * 0.05)) : 0; } diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java index 41c3a69e..c9a4a48d 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/tasks/LeaguePointsUpdateTask.java @@ -39,8 +39,8 @@ public void run() { } /** - * Returns a consumer that updates the ranking points of a user based on its leaderboard entry. - * @return + * Update ranking points of a user based on its leaderboard entry. + * @return {@code Consumer} that updates {@code leaguePoints} based on its leaderboard entry. */ private Consumer updateLeaderboardEntry() { return entry -> { @@ -52,8 +52,8 @@ private Consumer updateLeaderboardEntry() { } /** - * Returns the most recently completed leaderboard - * @return + * Retrieves the latest leaderboard based on the scheduled time of the environment. + * @return List of {@code LeaderboardEntryDTO} representing the latest leaderboard */ private List getLatestLeaderboard() { String[] timeParts = scheduledTime.split(":"); From 5409312fe679aca127092aafb110b540564ed7c2 Mon Sep 17 00:00:00 2001 From: GODrums Date: Mon, 2 Dec 2024 16:11:32 +0100 Subject: [PATCH 08/11] Add more JavaDocs --- .../leaderboard/LeaguePointsCalculationService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java index eaa82605..9a8abe08 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java @@ -31,7 +31,7 @@ public int calculateNewPoints(User user, LeaderboardEntryDTO entry) { // Apply minimum change to prevent extreme swings int newPoints = Math.max(0, oldPoints + pointChange); - logger.info("Points calculation: old={}, k={} decay={}, performanceBonus={}, placement={}, pointchange={}, new={}", + logger.info("Points calculation: old={}, k={}, decay={}, performanceBonus={}, placement={}, pointchange={}, new={}", oldPoints, kFactor, decay, performanceBonus, placementBonus, pointChange, newPoints); return newPoints; @@ -40,7 +40,7 @@ public int calculateNewPoints(User user, LeaderboardEntryDTO entry) { /** * Calculate the K factor for the user based on their current points and if they are a new player. * @param user - * @return + * @return K factor * @see Wikipedia: Most accurate K-factor */ private double getKFactor(User user) { @@ -58,7 +58,7 @@ private double getKFactor(User user) { /** * Check if the user's earliest merged pull request is within the last 30 days. * @param user - * @return + * @return true if the pull request is within the last 30 days */ private boolean isNewPlayer(User user) { return user.getMergedPullRequests().stream() @@ -70,7 +70,7 @@ private boolean isNewPlayer(User user) { /** * Calculate the base decay in points based on the current points. * @param currentPoints - * @return + * @return Amount of decay points */ private int calculateDecay(int currentPoints) { // 5% decay of current points, minimum 10 points if they have any points From e3e1212874ffd7036b77a4b66fd384ddc7c36c66 Mon Sep 17 00:00:00 2001 From: Armin Stanitzok Date: Tue, 3 Dec 2024 13:49:26 +0100 Subject: [PATCH 09/11] fix: new player condition --- .../hephaestus/leaderboard/LeaguePointsCalculationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java index 9a8abe08..bae5d9e8 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java @@ -64,7 +64,7 @@ private boolean isNewPlayer(User user) { return user.getMergedPullRequests().stream() .filter(PullRequest::isMerged) .map(PullRequest::getMergedAt) - .anyMatch(date -> date.isAfter(OffsetDateTime.now().minusDays(30))); + .noneMatch(date -> date.isAfter(OffsetDateTime.now().minusDays(30))); } /** From 1219d970ace2956c8a25328744be242ddc83f540 Mon Sep 17 00:00:00 2001 From: GODrums Date: Tue, 3 Dec 2024 18:27:45 +0100 Subject: [PATCH 10/11] Extract algorithm constants --- .../LeaguePointsCalculationService.java | 52 +++++++++++++++---- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java index bae5d9e8..abb9b1fc 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java @@ -11,10 +11,28 @@ public class LeaguePointsCalculationService { private final Logger logger = LoggerFactory.getLogger(LeaguePointsCalculationService.class); + // Starting points for new players + public static int POINTS_DEFAULT = 1000; + // Upper bound for first reducting in the k-factor + public static int POINTS_THRESHOLD_HIGH = 1750; + // Lower bound for first reducting in the k-factor + public static int POINTS_THRESHOLD_LOW = 1250; + // Minimum amount of points to decay each cycle + public static int DECAY_MINIMUM = 10; + // Factor to determine how much of the current points are decayed each cycle + public static double DECAY_FACTOR = 0.05; + // K-factors depending on the player's league points + public static double K_FACTOR_NEW_PLAYER = 2.0; + public static double K_FACTOR_LOW_POINTS = 1.5; + public static double K_FACTOR_MEDIUM_POINTS = 1.2; + public static double K_FACTOR_HIGH_POINTS = 1.1; + + + public int calculateNewPoints(User user, LeaderboardEntryDTO entry) { - // Initialize new players with 1000 points + // Initialize points for new players if (user.getLeaguePoints() == 0) { - user.setLeaguePoints(1000); + user.setLeaguePoints(POINTS_DEFAULT); } int oldPoints = user.getLeaguePoints(); @@ -39,19 +57,21 @@ public int calculateNewPoints(User user, LeaderboardEntryDTO entry) { /** * Calculate the K factor for the user based on their current points and if they are a new player. + * The K-factor is used to control the sensitivity of the rating system to changes in the leaderboard. + * New players have a higher K-factor to allow them to quickly reach their true skill level. * @param user * @return K factor * @see Wikipedia: Most accurate K-factor */ private double getKFactor(User user) { if (isNewPlayer(user)) { - return 2.0; - } else if (user.getLeaguePoints() < 1400) { - return 1.5; - } else if (user.getLeaguePoints() < 1800) { - return 1.2; + return K_FACTOR_NEW_PLAYER; + } else if (user.getLeaguePoints() < POINTS_THRESHOLD_LOW) { + return K_FACTOR_LOW_POINTS; + } else if (user.getLeaguePoints() < POINTS_THRESHOLD_HIGH) { + return K_FACTOR_MEDIUM_POINTS; } else { - return 1.1; + return K_FACTOR_HIGH_POINTS; } } @@ -69,19 +89,29 @@ private boolean isNewPlayer(User user) { /** * Calculate the base decay in points based on the current points. - * @param currentPoints + * @param currentPoints Current amount of league points * @return Amount of decay points */ private int calculateDecay(int currentPoints) { - // 5% decay of current points, minimum 10 points if they have any points - return currentPoints > 0 ? Math.max(10, (int)(currentPoints * 0.05)) : 0; + // decay a part of the current points, at least DECAY_MINIMUM points + return currentPoints > 0 ? Math.max(DECAY_MINIMUM, (int)(currentPoints * DECAY_FACTOR)) : 0; } + /** + * Calculate the bonus points based on the leaderboard score. + * @param score Leaderboard score + * @return Bonus points + */ private int calculatePerformanceBonus(int score) { // Convert leaderboard score directly to points with diminishing returns return (int)(Math.sqrt(score) * 10); } + /** + * Calculate the bonus points based on the placement in the leaderboard. + * @param placement Placement in the leaderboard + * @return Bonus points + */ private int calculatePlacementBonus(int placement) { // Bonus for top 3 placements return placement <= 3 ? 20 * (4 - placement) : 0; From 90061b953529d133fa2ae4c8ea7464ffbffd63f4 Mon Sep 17 00:00:00 2001 From: "Felix T.J. Dietrich" Date: Wed, 4 Dec 2024 20:08:02 +0100 Subject: [PATCH 11/11] Update server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java --- .../hephaestus/leaderboard/LeaguePointsCalculationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java index abb9b1fc..84d98fdc 100644 --- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java +++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaguePointsCalculationService.java @@ -47,7 +47,7 @@ public int calculateNewPoints(User user, LeaderboardEntryDTO entry) { // Calculate final point change int pointChange = (int) (kFactor * (performanceBonus + placementBonus - decay)); // Apply minimum change to prevent extreme swings - int newPoints = Math.max(0, oldPoints + pointChange); + int newPoints = Math.max(1, oldPoints + pointChange); logger.info("Points calculation: old={}, k={}, decay={}, performanceBonus={}, placement={}, pointchange={}, new={}", oldPoints, kFactor, decay, performanceBonus, placementBonus, pointChange, newPoints);