From 63c434d0306602b68c244fa63db202cfcff1dfba Mon Sep 17 00:00:00 2001
From: GODrums <armin.stanitzok@outlook.de>
Date: Sun, 22 Sep 2024 04:41:36 +0200
Subject: [PATCH] Time filter directly in SQL

---
 .../codereview/user/UserRepository.java           |  9 +++++++++
 .../hephaestus/codereview/user/UserService.java   |  6 ++++++
 .../leaderboard/LeaderboardService.java           | 15 +++++----------
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserRepository.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserRepository.java
index ebba4e74..ba998f9d 100644
--- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserRepository.java
+++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserRepository.java
@@ -1,5 +1,6 @@
 package de.tum.in.www1.hephaestus.codereview.user;
 
+import java.time.OffsetDateTime;
 import java.util.List;
 import java.util.Optional;
 
@@ -39,4 +40,12 @@ SELECT new UserDTO(u.id, u.login, u.email, u.name, u.url)
                 JOIN FETCH u.reviews
             """)
     List<User> findAllWithRelations();
+
+    @Query("""
+                SELECT u
+                FROM User u
+                JOIN FETCH u.reviews re
+                WHERE re.createdAt BETWEEN :after AND :before
+            """)
+    List<User> findAllInTimeframe(@Param("after") OffsetDateTime after, @Param("before") OffsetDateTime before);
 }
diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserService.java
index 1913c8ef..1a5f3e9b 100644
--- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserService.java
+++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/codereview/user/UserService.java
@@ -1,5 +1,6 @@
 package de.tum.in.www1.hephaestus.codereview.user;
 
+import java.time.OffsetDateTime;
 import java.util.List;
 import java.util.Optional;
 
@@ -31,4 +32,9 @@ public List<User> getAllUsers() {
         logger.info("Getting all users");
         return userRepository.findAll().stream().toList();
     }
+
+    public List<User> getAllUsersInTimeframe(OffsetDateTime after, OffsetDateTime before) {
+        logger.info("Getting all users in timeframe between " + after + " and " + before);
+        return userRepository.findAllInTimeframe(after, before);
+    }
 }
diff --git a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java
index 852ac6b7..fbb495f6 100644
--- a/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java
+++ b/server/application-server/src/main/java/de/tum/in/www1/hephaestus/leaderboard/LeaderboardService.java
@@ -41,16 +41,17 @@ public LeaderboardService(UserService userService) {
     public List<LeaderboardEntry> createLeaderboard(Optional<LocalDate> after, Optional<LocalDate> before) {
         logger.info("Creating leaderboard dataset");
 
-        List<User> users = userService.getAllUsers();
-
         LocalDateTime afterCutOff = after.isPresent() ? after.get().atStartOfDay()
                 : LocalDate.now().minusDays(timeframe).atStartOfDay();
         Optional<LocalDateTime> beforeCutOff = before.map(date -> date.plusDays(1).atStartOfDay());
 
-        logger.info("Leaderboard has " + users.size() + " users with cut-off time " + afterCutOff + " and before time "
-                + beforeCutOff);
+        List<User> users = userService.getAllUsersInTimeframe(afterCutOff.atOffset(ZoneOffset.UTC),
+                beforeCutOff.map(b -> b.atOffset(ZoneOffset.UTC)).orElse(OffsetDateTime.now()));
+
+        logger.info("Found " + users.size() + " users for the leaderboard");
 
         List<LeaderboardEntry> leaderboard = users.stream().map(user -> {
+            // ignore non-users, e.g. bots
             if (user.getType() != UserType.USER) {
                 return null;
             }
@@ -60,7 +61,6 @@ public List<LeaderboardEntry> createLeaderboard(Optional<LocalDate> after, Optio
             Set<PullRequestReviewDTO> commentSet = new HashSet<>();
 
             user.getReviews().stream()
-                    .filter(review -> isInTimeframe(review.getCreatedAt(), afterCutOff, beforeCutOff))
                     .forEach(review -> {
                         if (review.getPullRequest().getAuthor().getLogin().equals(user.getLogin())) {
                             return;
@@ -104,11 +104,6 @@ public List<LeaderboardEntry> createLeaderboard(Optional<LocalDate> after, Optio
         return leaderboard;
     }
 
-    private boolean isInTimeframe(OffsetDateTime date, LocalDateTime after, Optional<LocalDateTime> before) {
-        return date != null && (before.isPresent() && date.isBefore(before.get().atOffset(ZoneOffset.UTC))
-                || date.isAfter(after.atOffset(ZoneOffset.UTC)));
-    }
-
     /**
      * Calculates the score for a given pull request.
      * Possible values: 1, 3, 7, 17, 33.