From 37476fd8f9ea35cab77afc6e3aa15802447db2c8 Mon Sep 17 00:00:00 2001 From: Vankka Date: Sun, 18 Jun 2023 22:36:59 +0300 Subject: [PATCH] Fix group sync not finding members that are not cached, enable chunking by default. Add GroupSyncResult for being able to interact with role --- .../discord/entity/guild/DiscordGuild.java | 7 + .../entity/guild/DiscordGuildMember.java | 7 + .../config/main/MemberCachingConfig.java | 2 +- .../api/entity/guild/DiscordGuildImpl.java | 5 + .../entity/guild/DiscordGuildMemberImpl.java | 5 + .../common/groupsync/GroupSyncModule.java | 211 +++++++++--------- .../groupsync/enums/GroupSyncResult.java | 1 + 7 files changed, 136 insertions(+), 102 deletions(-) diff --git a/api/src/main/java/com/discordsrv/api/discord/entity/guild/DiscordGuild.java b/api/src/main/java/com/discordsrv/api/discord/entity/guild/DiscordGuild.java index ddf3449a..5ba828f8 100644 --- a/api/src/main/java/com/discordsrv/api/discord/entity/guild/DiscordGuild.java +++ b/api/src/main/java/com/discordsrv/api/discord/entity/guild/DiscordGuild.java @@ -54,6 +54,13 @@ public interface DiscordGuild extends JDAEntity, Snowflake { @Placeholder("server_member_count") int getMemberCount(); + /** + * Gets the bot's membership in the server. + * @return the connected bot's member + */ + @NotNull + DiscordGuildMember getSelfMember(); + /** * Retrieves a Discord guild member from Discord by id. * @param id the id for the Discord guild member diff --git a/api/src/main/java/com/discordsrv/api/discord/entity/guild/DiscordGuildMember.java b/api/src/main/java/com/discordsrv/api/discord/entity/guild/DiscordGuildMember.java index ea29c8e8..10f04dab 100644 --- a/api/src/main/java/com/discordsrv/api/discord/entity/guild/DiscordGuildMember.java +++ b/api/src/main/java/com/discordsrv/api/discord/entity/guild/DiscordGuildMember.java @@ -76,6 +76,13 @@ public interface DiscordGuildMember extends JDAEntity, Mentionable { */ boolean hasRole(@NotNull DiscordRole role); + /** + * If this member can interact (edit, add/take from members) with the specified role. + * @param role the role + * @return {@code true} if the member has a role above the specified role or is the server owner + */ + boolean canInteract(@NotNull DiscordRole role); + /** * Gives the given role to this member. * @param role the role to give diff --git a/common/src/main/java/com/discordsrv/common/config/main/MemberCachingConfig.java b/common/src/main/java/com/discordsrv/common/config/main/MemberCachingConfig.java index 301eae4f..e0121056 100644 --- a/common/src/main/java/com/discordsrv/common/config/main/MemberCachingConfig.java +++ b/common/src/main/java/com/discordsrv/common/config/main/MemberCachingConfig.java @@ -34,7 +34,7 @@ public class MemberCachingConfig { public boolean all = false; @Comment("If members should be cached at startup, this requires the \"Server Members Intent\"") - public boolean chunk = false; + public boolean chunk = true; @Comment("Filter for which servers should be cached at startup") public GuildFilter chunkingServerFilter = new GuildFilter(); diff --git a/common/src/main/java/com/discordsrv/common/discord/api/entity/guild/DiscordGuildImpl.java b/common/src/main/java/com/discordsrv/common/discord/api/entity/guild/DiscordGuildImpl.java index a187f4b8..8415c184 100644 --- a/common/src/main/java/com/discordsrv/common/discord/api/entity/guild/DiscordGuildImpl.java +++ b/common/src/main/java/com/discordsrv/common/discord/api/entity/guild/DiscordGuildImpl.java @@ -56,6 +56,11 @@ public class DiscordGuildImpl implements DiscordGuild { return guild.getMemberCount(); } + @Override + public @NotNull DiscordGuildMember getSelfMember() { + return discordSRV.discordAPI().getGuildMember(guild.getSelfMember()); + } + @Override public @NotNull CompletableFuture retrieveMemberById(long id) { return discordSRV.discordAPI().mapExceptions(() -> guild.retrieveMemberById(id) diff --git a/common/src/main/java/com/discordsrv/common/discord/api/entity/guild/DiscordGuildMemberImpl.java b/common/src/main/java/com/discordsrv/common/discord/api/entity/guild/DiscordGuildMemberImpl.java index 22b86fed..30fb9bc3 100644 --- a/common/src/main/java/com/discordsrv/common/discord/api/entity/guild/DiscordGuildMemberImpl.java +++ b/common/src/main/java/com/discordsrv/common/discord/api/entity/guild/DiscordGuildMemberImpl.java @@ -87,6 +87,11 @@ public class DiscordGuildMemberImpl implements DiscordGuildMember { return roles.stream().anyMatch(role::equals); } + @Override + public boolean canInteract(@NotNull DiscordRole role) { + return member.canInteract(role.asJDA()); + } + @Override public CompletableFuture addRole(@NotNull DiscordRole role) { return discordSRV.discordAPI().mapExceptions(() -> diff --git a/common/src/main/java/com/discordsrv/common/groupsync/GroupSyncModule.java b/common/src/main/java/com/discordsrv/common/groupsync/GroupSyncModule.java index aeb1477a..7e69c2bc 100644 --- a/common/src/main/java/com/discordsrv/common/groupsync/GroupSyncModule.java +++ b/common/src/main/java/com/discordsrv/common/groupsync/GroupSyncModule.java @@ -18,7 +18,6 @@ package com.discordsrv.common.groupsync; -import com.discordsrv.api.discord.entity.guild.DiscordGuildMember; import com.discordsrv.api.discord.entity.guild.DiscordRole; import com.discordsrv.api.discord.events.member.role.DiscordMemberRoleAddEvent; import com.discordsrv.api.discord.events.member.role.DiscordMemberRoleRemoveEvent; @@ -316,85 +315,90 @@ public class GroupSyncModule extends AbstractModule { return CompletableFuture.completedFuture(GroupSyncResult.ROLE_DOESNT_EXIST); } - DiscordGuildMember member = role.getGuild().getMemberById(userId); - if (member == null) { - return CompletableFuture.completedFuture(GroupSyncResult.NOT_A_GUILD_MEMBER); + if (!role.getGuild().getSelfMember().canInteract(role)) { + return CompletableFuture.completedFuture(GroupSyncResult.ROLE_CANNOT_INTERACT); } - boolean hasRole = member.hasRole(role); - String groupName = pair.groupName; - CompletableFuture resultFuture = new CompletableFuture<>(); - - hasGroup(player, groupName, pair.serverContext).whenComplete((hasGroup, t) -> { - if (t != null) { - discordSRV.logger().error("Failed to check if player " + player + " has group " + groupName, t); - resultFuture.complete(GroupSyncResult.PERMISSION_BACKEND_FAIL_CHECK); - return; + return role.getGuild().retrieveMemberById(userId).thenCompose(member -> { + if (member == null) { + return CompletableFuture.completedFuture(GroupSyncResult.NOT_A_GUILD_MEMBER); } - if (hasRole == hasGroup) { - resultFuture.complete(hasRole ? GroupSyncResult.BOTH_TRUE : GroupSyncResult.BOTH_FALSE); - // We're all good - return; - } + boolean hasRole = member.hasRole(role); + String groupName = pair.groupName; + CompletableFuture resultFuture = new CompletableFuture<>(); - GroupSyncSide side = pair.tieBreaker(); - GroupSyncDirection direction = pair.direction(); - CompletableFuture future; - GroupSyncResult result; - if (hasRole) { - if (side == GroupSyncSide.DISCORD) { - // Has role, add group - if (direction == GroupSyncDirection.MINECRAFT_TO_DISCORD) { - resultFuture.complete(GroupSyncResult.WRONG_DIRECTION); - return; - } - - result = GroupSyncResult.ADD_GROUP; - future = addGroup(player, groupName, pair.serverContext); - } else { - // Doesn't have group, remove role - if (direction == GroupSyncDirection.DISCORD_TO_MINECRAFT) { - resultFuture.complete(GroupSyncResult.WRONG_DIRECTION); - return; - } - - result = GroupSyncResult.REMOVE_ROLE; - future = member.removeRole(role); - } - } else { - if (side == GroupSyncSide.DISCORD) { - // Doesn't have role, remove group - if (direction == GroupSyncDirection.MINECRAFT_TO_DISCORD) { - resultFuture.complete(GroupSyncResult.WRONG_DIRECTION); - return; - } - - result = GroupSyncResult.REMOVE_GROUP; - future = removeGroup(player, groupName, pair.serverContext); - } else { - // Has group, add role - if (direction == GroupSyncDirection.DISCORD_TO_MINECRAFT) { - resultFuture.complete(GroupSyncResult.WRONG_DIRECTION); - return; - } - - result = GroupSyncResult.ADD_ROLE; - future = member.addRole(role); - } - } - future.whenComplete((v, t2) -> { - if (t2 != null) { - discordSRV.logger().error("Failed to " + result + " to " + player + "/" + Long.toUnsignedString(userId), t2); - resultFuture.complete(GroupSyncResult.UPDATE_FAILED); + hasGroup(player, groupName, pair.serverContext).whenComplete((hasGroup, t) -> { + if (t != null) { + discordSRV.logger().error("Failed to check if player " + player + " has group " + groupName, t); + resultFuture.complete(GroupSyncResult.PERMISSION_BACKEND_FAIL_CHECK); return; } - resultFuture.complete(result); - }); - }); + if (hasRole == hasGroup) { + resultFuture.complete(hasRole ? GroupSyncResult.BOTH_TRUE : GroupSyncResult.BOTH_FALSE); + // We're all good + return; + } - return resultFuture; + GroupSyncSide side = pair.tieBreaker(); + GroupSyncDirection direction = pair.direction(); + CompletableFuture future; + GroupSyncResult result; + if (hasRole) { + if (side == GroupSyncSide.DISCORD) { + // Has role, add group + if (direction == GroupSyncDirection.MINECRAFT_TO_DISCORD) { + resultFuture.complete(GroupSyncResult.WRONG_DIRECTION); + return; + } + + result = GroupSyncResult.ADD_GROUP; + future = addGroup(player, groupName, pair.serverContext); + } else { + // Doesn't have group, remove role + if (direction == GroupSyncDirection.DISCORD_TO_MINECRAFT) { + resultFuture.complete(GroupSyncResult.WRONG_DIRECTION); + return; + } + + result = GroupSyncResult.REMOVE_ROLE; + future = member.removeRole(role); + } + } else { + if (side == GroupSyncSide.DISCORD) { + // Doesn't have role, remove group + if (direction == GroupSyncDirection.MINECRAFT_TO_DISCORD) { + resultFuture.complete(GroupSyncResult.WRONG_DIRECTION); + return; + } + + result = GroupSyncResult.REMOVE_GROUP; + future = removeGroup(player, groupName, pair.serverContext); + } else { + // Has group, add role + if (direction == GroupSyncDirection.DISCORD_TO_MINECRAFT) { + resultFuture.complete(GroupSyncResult.WRONG_DIRECTION); + return; + } + + result = GroupSyncResult.ADD_ROLE; + future = member.addRole(role); + } + } + future.whenComplete((v, t2) -> { + if (t2 != null) { + discordSRV.logger().error("Failed to " + result + " to " + player + "/" + Long.toUnsignedString(userId), t2); + resultFuture.complete(GroupSyncResult.UPDATE_FAILED); + return; + } + + resultFuture.complete(result); + }); + }); + + return resultFuture; + }); } // Listeners & methods to indicate something changed @@ -636,44 +640,49 @@ public class GroupSyncModule extends AbstractModule { return CompletableFuture.completedFuture(GroupSyncResult.ROLE_DOESNT_EXIST); } - DiscordGuildMember member = role.getGuild().getMemberById(userId); - if (member == null) { - return CompletableFuture.completedFuture(GroupSyncResult.NOT_A_GUILD_MEMBER); + if (!role.getGuild().getSelfMember().canInteract(role)) { + return CompletableFuture.completedFuture(GroupSyncResult.ROLE_CANNOT_INTERACT); } - Map expected = expectedDiscordChanges.get(userId, key -> new ConcurrentHashMap<>()); - if (expected != null) { - expected.put(roleId, remove); - } - - boolean hasRole = member.hasRole(role); - CompletableFuture future; - if (remove && hasRole) { - future = member.removeRole(role).thenApply(v -> GroupSyncResult.REMOVE_ROLE); - } else if (!remove && !hasRole) { - future = member.addRole(role).thenApply(v -> GroupSyncResult.ADD_ROLE); - } else { - if (expected != null) { - // Nothing needed to be changed, remove expectation - expected.remove(roleId); + return role.getGuild().retrieveMemberById(userId).thenCompose(member -> { + if (member == null) { + return CompletableFuture.completedFuture(GroupSyncResult.NOT_A_GUILD_MEMBER); } - return CompletableFuture.completedFuture(GroupSyncResult.ALREADY_IN_SYNC); - } - CompletableFuture resultFuture = new CompletableFuture<>(); - future.whenComplete((result, t) -> { - if (t != null) { + Map expected = expectedDiscordChanges.get(userId, key -> new ConcurrentHashMap<>()); + if (expected != null) { + expected.put(roleId, remove); + } + + boolean hasRole = member.hasRole(role); + CompletableFuture future; + if (remove && hasRole) { + future = member.removeRole(role).thenApply(v -> GroupSyncResult.REMOVE_ROLE); + } else if (!remove && !hasRole) { + future = member.addRole(role).thenApply(v -> GroupSyncResult.ADD_ROLE); + } else { if (expected != null) { - // Failed, remove expectation + // Nothing needed to be changed, remove expectation expected.remove(roleId); } - - resultFuture.complete(GroupSyncResult.UPDATE_FAILED); - discordSRV.logger().error("Failed to give/take role " + role + " to/from " + member, t); - return; + return CompletableFuture.completedFuture(GroupSyncResult.ALREADY_IN_SYNC); } - resultFuture.complete(result); + + CompletableFuture resultFuture = new CompletableFuture<>(); + future.whenComplete((result, t) -> { + if (t != null) { + if (expected != null) { + // Failed, remove expectation + expected.remove(roleId); + } + + resultFuture.complete(GroupSyncResult.UPDATE_FAILED); + discordSRV.logger().error("Failed to give/take role " + role + " to/from " + member, t); + return; + } + resultFuture.complete(result); + }); + return resultFuture; }); - return resultFuture; } } diff --git a/common/src/main/java/com/discordsrv/common/groupsync/enums/GroupSyncResult.java b/common/src/main/java/com/discordsrv/common/groupsync/enums/GroupSyncResult.java index 0e37720a..bdc96aa3 100644 --- a/common/src/main/java/com/discordsrv/common/groupsync/enums/GroupSyncResult.java +++ b/common/src/main/java/com/discordsrv/common/groupsync/enums/GroupSyncResult.java @@ -34,6 +34,7 @@ public enum GroupSyncResult { // Errors ROLE_DOESNT_EXIST("Role doesn't exist"), + ROLE_CANNOT_INTERACT("Bot doesn't have a role above the synced role (cannot interact)"), NOT_A_GUILD_MEMBER("User is not part of the server the role is in"), PERMISSION_BACKEND_FAIL_CHECK("Failed to check group status, error printed"), UPDATE_FAILED("Failed to modify role/group, error printed"),