Address code review comments from @SquidXTV

  - CakeDayListener.java: remove constructor JavaDoc
  - CakeDayRoutine.java: remove constructor JavaDoc
  - perf(findUserCakeDayFromDatabase): return one from DB
  - perf(CakeDayService): remove unnecessary call
  - Rename from handleUserLeft to removeUserCakeDay
  - perf(cakeDayRole): delegate role fetching to JDA
  - CakeDayService: make createMemberCakeDayQuery static
  - perf(CakeDayService): remove unneeded batch() call
  - CakeDayService: async insert member cake day to db
  - CakeDayService.java: rewrite removeRoleFromMembers()
  - perf: use Role that we have fetched for refreshing
  - CakeDayService: remove unnecessary call to get snowflake
  - perf: remove redundant check for if it's cake day
  - CakeDayService.java: simplify Optional unwrapping
  - CakeDayService.java: rename config to fullConfig
  - docs(CakeDayConfig): clarify with `@see`

Signed-off-by: Chris Sdogkos <[email protected]>

Author: christolis

application/src/main/java/org/togetherjava/tjbot/config/CakeDayConfig.java

@@ -6,6 +6,8 @@
 
 /**
  * Configuration record for the Cake Day feature.
+ *
+ * @see org.togetherjava.tjbot.features.cakeday.CakeDayService
  */
 public record CakeDayConfig(
         @JsonProperty(value = "rolePattern", required = true) String rolePattern) {

application/src/main/java/org/togetherjava/tjbot/features/cakeday/CakeDayListener.java

@@ -20,11 +20,6 @@ public class CakeDayListener extends ListenerAdapter implements EventReceiver {
 
     private final CakeDayService cakeDayService;
 
-    /**
-     * Constructs a new CakeDayListener with the given {@link CakeDayService}.
-     *
-     * @param cakeDayService the {@link CakeDayService} to be used by this listener
-     */
     public CakeDayListener(CakeDayService cakeDayService) {
         this.cakeDayService = cakeDayService;
     }
@@ -79,6 +74,6 @@ public void onGuildMemberRemove(GuildMemberRemoveEvent event) {
         User user = event.getUser();
         Guild guild = event.getGuild();
 
-        cakeDayService.handleUserLeft(user, guild);
+        cakeDayService.removeUserCakeDay(user, guild);
     }
 }

application/src/main/java/org/togetherjava/tjbot/features/cakeday/CakeDayRoutine.java

@@ -17,11 +17,6 @@ public class CakeDayRoutine implements Routine {
 
     private final CakeDayService cakeDayService;
 
-    /**
-     * Constructs a new {@link CakeDayRoutine} instance.
-     *
-     * @param cakeDayService an instance of the cake day service
-     */
     public CakeDayRoutine(CakeDayService cakeDayService) {
         this.cakeDayService = cakeDayService;
     }

application/src/main/java/org/togetherjava/tjbot/features/cakeday/CakeDayService.java

@@ -4,9 +4,7 @@
 import net.dv8tion.jda.api.entities.Member;
 import net.dv8tion.jda.api.entities.Role;
 import net.dv8tion.jda.api.entities.User;
-import net.dv8tion.jda.api.entities.UserSnowflake;
-import net.dv8tion.jda.api.requests.RestAction;
-import net.dv8tion.jda.api.utils.Result;
+import org.jetbrains.annotations.NotNull;
 import org.jooq.Query;
 import org.jooq.impl.DSL;
 import org.slf4j.Logger;
@@ -23,9 +21,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.Predicate;
-import java.util.regex.Pattern;
-import java.util.stream.Collectors;
 
 import static org.togetherjava.tjbot.db.generated.tables.CakeDays.CAKE_DAYS;
 
@@ -37,29 +32,29 @@ public class CakeDayService {
     private static final DateTimeFormatter MONTH_DAY_FORMATTER =
             DateTimeFormatter.ofPattern("MM-dd");
     private final Set<String> cakeDaysCache = new HashSet<>();
-    private final Predicate<String> cakeDayRolePredicate;
-    private final CakeDayConfig config;
+    private final String cakeDayRolePattern;
+    private final CakeDayConfig fullConfig;
     private final Database database;
 
     /**
      * Constructs a {@link CakeDayService} with the given configuration and database.
      *
-     * @param config the configuration for cake day management
+     * @param fullConfig the full configuration for cake day management
      * @param database the database for storing cake day information
      */
-    public CakeDayService(Config config, Database database) {
-        this.config = config.getCakeDayConfig();
+    public CakeDayService(Config fullConfig, Database database) {
+        this.fullConfig = fullConfig.getCakeDayConfig();
         this.database = database;
 
-        this.cakeDayRolePredicate = Pattern.compile(this.config.rolePattern()).asPredicate();
+        this.cakeDayRolePattern = this.fullConfig.rolePattern();
     }
 
     private Optional<Role> getCakeDayRole(Guild guild) {
         Optional<Role> cakeDayRole = getCakeDayRoleFromGuild(guild);
 
         if (cakeDayRole.isEmpty()) {
             logger.warn("Cake day role with pattern {} not found for guild: {}",
-                    config.rolePattern(), guild.getName());
+                    fullConfig.rolePattern(), guild.getName());
         }
 
         return cakeDayRole;
@@ -74,13 +69,7 @@ private Optional<Role> getCakeDayRole(Guild guild) {
      * @param guild the guild for which to reassign the cake day role
      */
     protected void reassignCakeDayRole(Guild guild) {
-        Optional<Role> cakeDayRole = getCakeDayRole(guild);
-
-        if (cakeDayRole.isEmpty()) {
-            return;
-        }
-
-        refreshMembersCakeDayRoles(cakeDayRole.get(), guild);
+        getCakeDayRole(guild).ifPresent(role -> refreshMembersCakeDayRoles(role, guild));
     }
 
     /**
@@ -92,7 +81,7 @@ protected void reassignCakeDayRole(Guild guild) {
     private void refreshMembersCakeDayRoles(Role cakeDayRole, Guild guild) {
         guild.findMembersWithRoles(cakeDayRole).onSuccess(members -> {
             removeRoleFromMembers(guild, cakeDayRole, members);
-            addTodayMembersCakeDayRole(guild);
+            addTodayMembersCakeDayRole(guild, cakeDayRole);
         });
     }
 
@@ -106,99 +95,96 @@ private void refreshMembersCakeDayRoles(Role cakeDayRole, Guild guild) {
      *
      * @param guild the guild to check for members celebrating their cake day today
      */
-    private void addTodayMembersCakeDayRole(Guild guild) {
+    private void addTodayMembersCakeDayRole(Guild guild, Role cakeDayRole) {
         findCakeDaysTodayFromDatabase(guild).forEach(cakeDayRecord -> {
             Member member = guild.getMemberById(cakeDayRecord.getUserId());
 
             if (member == null) {
                 return;
             }
 
-            boolean isAnniversaryDay = hasMemberCakeDayToday(member);
-            int yearsSinceJoin = OffsetDateTime.now().getYear() - cakeDayRecord.getJoinedYear();
-
-            if (yearsSinceJoin > 0 && isAnniversaryDay) {
-                addCakeDayRole(member);
+            if (!hasMemberCakeDayToday(member)) {
+                return;
             }
+
+            addCakeDayRole(member, cakeDayRole);
         });
     }
 
     /**
-     * Adds the cake day role to the specified member if the cake day role exists in the guild.
+     * Adds the cake day role supplied to the specified member if the cake day role exists in the
+     * guild.
      *
      * @param member the {@link Member} to whom the cake day role will be added
      */
     protected void addCakeDayRole(Member member) {
         Guild guild = member.getGuild();
-        UserSnowflake snowflake = UserSnowflake.fromId(member.getId());
         Optional<Role> cakeDayRole = getCakeDayRole(guild);
 
         if (cakeDayRole.isEmpty()) {
             return;
         }
 
-        guild.addRoleToMember(snowflake, cakeDayRole.get()).complete();
+        addCakeDayRole(member, cakeDayRole.get());
     }
 
     /**
-     * Removes a specified role from a list of members in a {@link Guild}.
+     * Adds the cake day role supplied to the specified member.
      *
-     * @param guild the {@link Guild} from which to remove the role from members
-     * @param role the {@link Role} to be removed from the members
-     * @param members the {@link List} of members from which the {@link Role} will be removed
+     * @param member the {@link Member} to whom the cake day role will be added
+     * @param cakeDayRole the cake day {@link Role}
      */
-    private void removeRoleFromMembers(Guild guild, Role role, List<Member> members) {
-        List<RestAction<Result<Void>>> chain = members.stream()
-            .map(member -> guild.removeRoleFromMember(member, role).mapToResult())
-            .toList();
-
-        if (chain.isEmpty()) {
-            return;
-        }
+    private void addCakeDayRole(Member member, @NotNull Role cakeDayRole) {
+        Guild guild = member.getGuild();
 
-        RestAction.allOf(chain).queue();
+        guild.addRoleToMember(member, cakeDayRole).queue();
     }
 
     /**
-     * Creates a query to insert a member's cake day information into the database.
+     * Removes a specified role from a list of members in a {@link Guild}.
      *
-     * @param member the member whose cake day information is to be inserted
-     * @param guildId the ID of the guild to which the member belongs
-     * @return an Optional containing the query to insert cake day information if the member has a
-     *         join time; empty Optional otherwise
+     * @param guild the {@link Guild} from which to remove the role from members
+     * @param role the {@link Role} to be removed from the members
+     * @param members the {@link List} of members from which the {@link Role} will be removed
      */
-    private Optional<Query> createMemberCakeDayQuery(Member member, long guildId) {
-        if (!member.hasTimeJoined()) {
-            return Optional.empty();
-        }
-
-        OffsetDateTime cakeDay = member.getTimeJoined();
-        String joinedMonthDay = cakeDay.format(MONTH_DAY_FORMATTER);
-
-        return Optional.of(DSL.insertInto(CAKE_DAYS)
-            .set(CAKE_DAYS.JOINED_MONTH_DAY, joinedMonthDay)
-            .set(CAKE_DAYS.JOINED_YEAR, cakeDay.getYear())
-            .set(CAKE_DAYS.GUILD_ID, guildId)
-            .set(CAKE_DAYS.USER_ID, member.getIdLong()));
+    private void removeRoleFromMembers(Guild guild, Role role, List<Member> members) {
+        members.forEach(member -> guild.removeRoleFromMember(member, role)
+            .queue(null,
+                    failure -> logger.error("Could not remove role {} from {} ({}): {}",
+                            role.getName(), member.getEffectiveName(), member.getId(),
+                            failure.getMessage())));
     }
 
     /**
-     * Inserts the cake day of a member into the database.
+     * Asynchronously inserts a member's cake day information into the database.
+     *
+     * <p>
+     * This method retrieves the {@link Member} from the Discord API to obtain the member's join
+     * timestamp, derives the month/day portion using {@code MONTH_DAY_FORMATTER}, and then inserts
+     * a row into {@code CAKE_DAYS} with the join month-day, join year, guild id, and user id.
+     *
      * <p>
-     * If the member has no join date, nothing happens.
+     * The Discord retrieval is performed asynchronously; on success the database write is executed.
+     * If the Discord retrieval fails (e.g., member not found, missing permissions, network issues),
+     * the insert is skipped and a warning is logged.
      *
-     * @param member the member whose cake day is to be inserted into the database
-     * @param guildId the ID of the guild to which the member belongs
+     * @param member the member whose cake day information should be inserted
+     * @param guildId the id of the guild associated with the cake day record
      */
     protected void insertMemberCakeDayToDatabase(Member member, long guildId) {
-        Query insertQuery = createMemberCakeDayQuery(member, guildId).orElse(null);
-
-        if (insertQuery == null) {
-            logger.warn("Tried to add member {} to database but found no time joined",
-                    member.getId());
-        }
-
-        database.write(context -> context.batch(insertQuery).execute());
+        member.getGuild().retrieveMemberById(member.getIdLong()).queue(retrievedMember -> {
+            OffsetDateTime timeJoined = retrievedMember.getTimeJoined();
+            String joinedMonthDay = timeJoined.format(MONTH_DAY_FORMATTER);
+
+            Query query = DSL.insertInto(CAKE_DAYS)
+                .set(CAKE_DAYS.JOINED_MONTH_DAY, joinedMonthDay)
+                .set(CAKE_DAYS.JOINED_YEAR, timeJoined.getYear())
+                .set(CAKE_DAYS.GUILD_ID, guildId)
+                .set(CAKE_DAYS.USER_ID, member.getIdLong());
+
+            database.write(ctx -> ctx.execute(query));
+        }, failure -> logger.warn("Could not insert member cake day into the database: {}",
+                failure.getMessage()));
     }
 
     /**
@@ -221,10 +207,7 @@ protected void removeMemberCakeDayFromDatabase(long userId, long guildId) {
      * @return an {@link Optional} containing the cake day role if found, otherwise empty
      */
     private Optional<Role> getCakeDayRoleFromGuild(Guild guild) {
-        return guild.getRoles()
-            .stream()
-            .filter(role -> cakeDayRolePredicate.test(role.getName()))
-            .findFirst();
+        return guild.getRolesByName(cakeDayRolePattern, true).stream().findFirst();
     }
 
     /**
@@ -234,7 +217,7 @@ private Optional<Role> getCakeDayRoleFromGuild(Guild guild) {
      * @param user the {@link User} who left the guild
      * @param guild the {@link Guild} from which the user left
      */
-    protected void handleUserLeft(User user, Guild guild) {
+    protected void removeUserCakeDay(User user, Guild guild) {
         removeMemberCakeDayFromDatabase(user.getIdLong(), guild.getIdLong());
         cakeDaysCache.remove(guild.getId());
     }
@@ -247,12 +230,10 @@ protected void handleUserLeft(User user, Guild guild) {
     private List<CakeDaysRecord> findCakeDaysTodayFromDatabase(Guild guild) {
         String todayMonthDay = OffsetDateTime.now().format(MONTH_DAY_FORMATTER);
 
-        return database
-            .read(context -> context.selectFrom(CAKE_DAYS)
-                .where(CAKE_DAYS.JOINED_MONTH_DAY.eq(todayMonthDay))
-                .and(CAKE_DAYS.GUILD_ID.eq(guild.getIdLong()))
-                .fetch())
-            .collect(Collectors.toList());
+        return database.read(context -> context.selectFrom(CAKE_DAYS)
+            .where(CAKE_DAYS.JOINED_MONTH_DAY.eq(todayMonthDay))
+            .and(CAKE_DAYS.GUILD_ID.eq(guild.getIdLong()))
+            .fetch());
     }
 
     /**
@@ -263,13 +244,8 @@ private List<CakeDaysRecord> findCakeDaysTodayFromDatabase(Guild guild) {
      *         {@link Optional} if no record is found
      */
     protected Optional<CakeDaysRecord> findUserCakeDayFromDatabase(long userId) {
-        return database
-            .read(context -> context.selectFrom(CAKE_DAYS)
-                .where(CAKE_DAYS.USER_ID.eq(userId))
-                .fetch())
-            .collect(Collectors.toList())
-            .stream()
-            .findFirst();
+        return database.read(ctx -> Optional
+            .ofNullable(ctx.selectFrom(CAKE_DAYS).where(CAKE_DAYS.USER_ID.eq(userId)).fetchOne()));
     }
 
     /**