Skip to content

⚡ Optimize GuildCommand database calls #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
acsoto wants to merge 1 commit into
base: main
Choose a base branch
from
+228 −46
Open

⚡ Optimize GuildCommand database calls #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions modules/GuildManager/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
</repository>
<repository>
<id>placeholderapi</id>
<url>https://repo.extendedclip.com/content/repositories/placeholderapi/</url>
<url>https://repo.helpch.at/releases/</url>
</repository>
<repository>
<id>codemc-repo</id>
Expand All @@ -111,7 +111,7 @@
<dependency>
<groupId>me.clip</groupId>
<artifactId>placeholderapi</artifactId>
<version>2.11.1</version>
<version>2.11.5</version>
<scope>provided</scope>
</dependency>
<dependency>
Expand All @@ -120,6 +120,18 @@
<version>5.6.0-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>3.12.4</version>
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Mockito version 3.12.4 is from 2021 and is outdated. Consider upgrading to a more recent version like 4.11.0 or 5.x for better compatibility and features. Similarly, JUnit 4.13.2 is the last version of JUnit 4, but JUnit 5 (Jupiter) is the current standard and provides better features. Consider upgrading to JUnit 5 unless there are specific compatibility requirements.

Suggested change
<version>3.12.4</version>
<version>4.11.0</version>

Copilot uses AI. Check for mistakes.
<scope>test</scope>
</dependency>
</dependencies>


Expand Down
4 changes: 2 additions & 2 deletions modules/GuildManager/src/main/java/com/mcatk/guildmanager/GuildManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import org.bukkit.plugin.java.JavaPlugin;
import org.bukkit.plugin.RegisteredServiceProvider;

public final class GuildManager extends JavaPlugin {
public class GuildManager extends JavaPlugin {
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the 'final' modifier from the GuildManager class to facilitate mocking in tests is generally acceptable, but it weakens the class design by allowing unintended subclassing in production code. Consider using alternative approaches like extracting an interface or using PowerMock/Mockito inline mocking instead, which can mock final classes without modifying production code.

Suggested change
public class GuildManager extends JavaPlugin {
public final class GuildManager extends JavaPlugin {

Copilot uses AI. Check for mistakes.

private static GuildManager plugin;
private static Economy econ;
Expand Down Expand Up @@ -62,7 +62,7 @@ private void registerDependency() {

private void registerCommand() {
Bukkit.getPluginCommand("guildmanager").
setExecutor(new GuildCommand());
setExecutor(new GuildCommand(this, getGuildService()));
Bukkit.getPluginCommand("guildmanagers").
setExecutor(new GuildCommandS());
getLogger().info("注册指令注册完毕");
Expand Down
94 changes: 52 additions & 42 deletions modules/GuildManager/src/main/java/com/mcatk/guildmanager/command/GuildCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.mcatk.guildmanager.GuildManager;
import com.mcatk.guildmanager.Msg;
import com.mcatk.guildmanager.gui.GuildsGUI;
import com.mcatk.guildmanager.core.service.GuildService;
import com.mcatk.guildmanager.models.ApplicantsList;
import com.mcatk.guildmanager.models.Guild;
import com.mcatk.guildmanager.models.Member;
Expand All @@ -15,12 +16,16 @@

public class GuildCommand implements CommandExecutor {

private CommandSender sender;
private String[] args;
private Guild guild;
private final GuildManager plugin;
private final GuildService guildService;

public GuildCommand(GuildManager plugin, GuildService guildService) {
this.plugin = plugin;
this.guildService = guildService;
}

// usage: /gmg gui|apply|tp|create|t|quit|offer|msg|memgui|msggui
private void printHelp() {
private void printHelp(CommandSender sender) {
sender.sendMessage("§e------------公会帮助------------");
sender.sendMessage("§a/gmg gui §2公会列表");
sender.sendMessage("§a/gmg apply <ID> §2申请加入公会");
Expand All @@ -31,61 +36,67 @@ private void printHelp() {

@Override
public boolean onCommand(CommandSender sender, Command command, String label, String[] args) {
this.sender = sender;
this.args = args;
if (args.length == 0) {
printHelp();
} else {
this.guild = GuildManager.getPlugin().getGuildService().getPlayerGuild(sender.getName());
try {
onCommandWithoutGuild();
//以下要求发送者在一个公会之中
if (guild != null) {
onCommandWithGuild();
}
} catch (ParaLengthException e) {
sender.sendMessage("参数错误");
}
printHelp(sender);
return true;
}

// Optimization: Run DB lookup asynchronously
Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
Guild guild = guildService.getPlayerGuild(sender.getName());
Comment on lines +45 to +46
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid Bukkit API access on async thread

The async task now calls sender.getName() off the main thread. In Bukkit, most CommandSender implementations (especially Player) are not thread-safe, and accessing them asynchronously can trigger warnings or undefined behavior under async-catcher. This regression only occurs when a player runs the command and the async scheduler executes, so it’s input/context dependent. Capture the name on the main thread before scheduling, or pass a plain String into the async task.

Useful? React with 👍 / 👎.


// Switch back to sync thread for command processing
Bukkit.getScheduler().runTask(plugin, () -> {
try {
onCommandWithoutGuild(sender, args, guild);
//以下要求发送者在一个公会之中
if (guild != null) {
onCommandWithGuild(sender, args, guild);
}
} catch (ParaLengthException e) {
sender.sendMessage("参数错误");
}
});
Comment on lines +46 to +59
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If an exception occurs in the async task (line 46), it will not be caught and could cause the command to fail silently without feedback to the user. Consider wrapping the async operation in a try-catch block to handle potential exceptions (e.g., database connection failures) and provide appropriate error messages to the user via a sync callback.

Suggested change
Guild guild = guildService.getPlayerGuild(sender.getName());
// Switch back to sync thread for command processing
Bukkit.getScheduler().runTask(plugin, () -> {
try {
onCommandWithoutGuild(sender, args, guild);
//以下要求发送者在一个公会之中
if (guild != null) {
onCommandWithGuild(sender, args, guild);
}
} catch (ParaLengthException e) {
sender.sendMessage("参数错误");
}
});
try {
Guild guild = guildService.getPlayerGuild(sender.getName());
// Switch back to sync thread for command processing
Bukkit.getScheduler().runTask(plugin, () -> {
try {
onCommandWithoutGuild(sender, args, guild);
//以下要求发送者在一个公会之中
if (guild != null) {
onCommandWithGuild(sender, args, guild);
}
} catch (ParaLengthException e) {
sender.sendMessage("参数错误");
}
});
} catch (Exception e) {
// Handle unexpected errors in async task and notify the sender on the main thread
Bukkit.getScheduler().runTask(plugin, () -> {
sender.sendMessage("§c执行公会命令时发生错误，请稍后重试。");
});
e.printStackTrace();
}

Copilot uses AI. Check for mistakes.
});
return true;
}

private void onCommandWithoutGuild() throws ParaLengthException {
private void onCommandWithoutGuild(CommandSender sender, String[] args, Guild guild) throws ParaLengthException {
switch (args[0].toLowerCase()) {
case "gui":
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code casts CommandSender to Player without checking if the sender is actually a Player. If a console sender executes this command with the "gui" argument, this will throw a ClassCastException. Add a check before casting: if (!(sender instanceof Player)) { sender.sendMessage("This command can only be used by players"); return; }

Suggested change
case "gui":
case "gui":
if (!(sender instanceof Player)) {
sender.sendMessage("This command can only be used by players");
break;
}

Copilot uses AI. Check for mistakes.
new GuildsGUI().openGUI((Player) sender);
break;
case "apply":
apply();
apply(sender, args, guild);
break;
case "create":
create();
create(sender, args, guild);
break;
default:
}
}

private void onCommandWithGuild() throws ParaLengthException {
private void onCommandWithGuild(CommandSender sender, String[] args, Guild guild) throws ParaLengthException {
switch (args[0].toLowerCase()) {
case "offer":
offer();
offer(sender, args, guild);
break;
case "quit":
quit();
quit(sender, guild);
break;
}
}

private void apply() throws ParaLengthException {
private void apply(CommandSender sender, String[] args, Guild guild) throws ParaLengthException {
if (args.length != 2) {
throw new ParaLengthException(2);
}
if (GuildManager.getPlugin().getGuildService().getPlayerGuild(sender.getName()) != null) {
if (guild != null) {
sender.sendMessage(Msg.ERROR + "已有公会");
} else {
String guildID = args[1];
Guild guild = GuildManager.getPlugin().getGuildService().getGuild(guildID);
if (guild == null) {
Guild targetGuild = guildService.getGuild(guildID);
if (targetGuild == null) {
sender.sendMessage(Msg.ERROR + "不存在公会");
} else {
if (ApplicantsList.getApplicantsList().getList(guildID).contains(sender.getName())) {
Expand All @@ -98,7 +109,7 @@ private void apply() throws ParaLengthException {
}
}

private void create() throws ParaLengthException {
private void create(CommandSender sender, String[] args, Guild guild) throws ParaLengthException {
if (args.length != 2) {
throw new ParaLengthException(2);
}
Expand All @@ -107,7 +118,6 @@ private void create() throws ParaLengthException {
sender.sendMessage(Msg.ERROR + "ID只能是小写字母");
return;
}
Guild guild = GuildManager.getPlugin().getGuildService().getPlayerGuild(sender.getName());
if (guild != null) {
sender.sendMessage(Msg.ERROR + "你已在公会" + guild.getGuildName());
return;
Expand All @@ -116,17 +126,17 @@ private void create() throws ParaLengthException {
sender.sendMessage(Msg.ERROR + "§c该指令只能由玩家发出");
return;
}
if (GuildManager.getPlugin().takePlayerMoney((Player) sender, 1000000)) {
GuildManager.getPlugin().getGuildService().createGuild(guildID, sender.getName());
GuildManager.getPlugin().getGuildService().addMember(sender.getName(), guildID);
if (plugin.takePlayerMoney((Player) sender, 1000000)) {
guildService.createGuild(guildID, sender.getName());
guildService.addMember(sender.getName(), guildID);
Comment on lines +130 to +131
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines make blocking database calls (guildService.createGuild and guildService.addMember) on the main thread. According to the GuildService implementation, both methods are synchronized and call repository methods that perform database I/O, then call refresh() which also does database I/O. These operations should be moved to an async task to avoid blocking the main thread.

Suggested change
guildService.createGuild(guildID, sender.getName());
guildService.addMember(sender.getName(), guildID);
Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
guildService.createGuild(guildID, sender.getName());
guildService.addMember(sender.getName(), guildID);
});

Copilot uses AI. Check for mistakes.
sender.sendMessage(Msg.INFO + "创建成功");
GuildManager.getPlugin().getLogger().info("玩家" + sender.getName() + "创建了公会" + args[1]);
Bukkit.getLogger().info("玩家" + sender.getName() + "创建了公会" + args[1]);
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Bukkit.getLogger() instead of plugin.getLogger() changes the logger context from the plugin-specific logger to the global Minecraft logger. This makes it harder to identify which plugin generated the log message and may affect log filtering. Consider using plugin.getLogger() to maintain proper logging attribution, especially since the plugin instance is now available via the constructor-injected field.

Suggested change
Bukkit.getLogger().info("玩家" + sender.getName() + "创建了公会" + args[1]);
plugin.getLogger().info("玩家" + sender.getName() + "创建了公会" + args[1]);

Copilot uses AI. Check for mistakes.
} else {
sender.sendMessage(Msg.ERROR + "AC点不足！");
}
}

private void offer() throws ParaLengthException {
private void offer(CommandSender sender, String[] args, Guild guild) throws ParaLengthException {
String p = sender.getName();
if (args.length != 2) {
throw new ParaLengthException(2);
Expand All @@ -146,10 +156,10 @@ private void offer() throws ParaLengthException {
sender.sendMessage(Msg.INFO + "§c必须是10000的整数倍！");
return;
}
if (GuildManager.getPlugin().takePlayerMoney((Player) sender, n)) {
if (plugin.takePlayerMoney((Player) sender, n)) {
guild.setCash(guild.getCash() + n / 10000);
//add contribution and check if is full.
Member member = GuildManager.getPlugin().getGuildService().getMember(sender.getName());
Member member = guildService.getMember(sender.getName());
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line makes a blocking database call (guildService.getMember) on the main thread after the async operation completes. The getMember method in GuildService is synchronized and calls repository.getMember, which performs database I/O. This defeats the purpose of the async optimization. Move this database call to the async section along with other DB operations.

Copilot uses AI. Check for mistakes.
if (member.getContribution() + n / 10000 > 100) {
sender.sendMessage(Msg.INFO + "您的贡献值已满，无法继续增长");
} else {
Expand All @@ -159,19 +169,19 @@ private void offer() throws ParaLengthException {
Msg.INFO + "§a成功为" + guild.getGuildName() +
"§a捐赠" + n + "AC" + "折合为" + (n / 10000) + "公会资金"
);
GuildManager.getPlugin().getLogger().info(p + "捐献了" + n + "给" + guild.getGuildName());
GuildManager.getPlugin().getGuildService().saveMember(member);
GuildManager.getPlugin().getGuildService().saveGuild(guild);
Bukkit.getLogger().info(p + "捐献了" + n + "给" + guild.getGuildName());
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Bukkit.getLogger() instead of plugin.getLogger() changes the logger context from the plugin-specific logger to the global Minecraft logger. This makes it harder to identify which plugin generated the log message and may affect log filtering. Consider using plugin.getLogger() to maintain proper logging attribution, especially since the plugin instance is now available via the constructor-injected field.

Suggested change
Bukkit.getLogger().info(p + "捐献了" + n + "给" + guild.getGuildName());
plugin.getLogger().info(p + "捐献了" + n + "给" + guild.getGuildName());

Copilot uses AI. Check for mistakes.
guildService.saveMember(member);
guildService.saveGuild(guild);
Comment on lines +173 to +174
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines make blocking database calls (guildService.saveMember and guildService.saveGuild) on the main thread. Both methods are synchronized and perform database I/O operations followed by a refresh() call that also does database I/O. These operations should be moved to an async task to avoid blocking the main thread.

Suggested change
guildService.saveMember(member);
guildService.saveGuild(guild);
Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
guildService.saveMember(member);
guildService.saveGuild(guild);
});

Copilot uses AI. Check for mistakes.
} else {
sender.sendMessage(Msg.ERROR + "AC点不足！");
}
}

private void quit() {
private void quit(CommandSender sender, Guild guild) {
if (guild.isManager(sender.getName())) {
sender.sendMessage("请先撤销你的公会职务");
} else {
GuildManager.getPlugin().getGuildService().removeMember(sender.getName(), guild.getId());
guildService.removeMember(sender.getName(), guild.getId());
Bukkit.dispatchCommand(Bukkit.getConsoleSender(), String.format("res pset main.gh %s move remove", sender.getName()));
sender.sendMessage(Msg.INFO + "退出公会" + guild.getGuildName());
Comment on lines +184 to 186
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line makes a blocking database call (guildService.removeMember) on the main thread. The removeMember method is synchronized and performs database I/O operations followed by a refresh() call that also does database I/O. This operation should be moved to an async task to avoid blocking the main thread.

Suggested change
guildService.removeMember(sender.getName(), guild.getId());
Bukkit.dispatchCommand(Bukkit.getConsoleSender(), String.format("res pset main.gh %s move remove", sender.getName()));
sender.sendMessage(Msg.INFO + "退出公会" + guild.getGuildName());
String playerName = sender.getName();
String guildId = guild.getId();
String guildName = guild.getGuildName();
Bukkit.getScheduler().runTaskAsynchronously(plugin, () -> {
guildService.removeMember(playerName, guildId);
Bukkit.getScheduler().runTask(plugin, () -> {
Bukkit.dispatchCommand(
Bukkit.getConsoleSender(),
String.format("res pset main.gh %s move remove", playerName)
);
sender.sendMessage(Msg.INFO + "退出公会" + guildName);
});
});

Copilot uses AI. Check for mistakes.
}
Expand Down
1 change: 1 addition & 0 deletions modules/GuildManager/src/main/java/com/mcatk/guildmanager/command/GuildCommandS.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.mcatk.guildmanager.command;

import com.mcatk.guildmanager.GuildManager;
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description states that thread safety issues were addressed by removing instance fields from GuildCommand, but GuildCommandS still has the same problematic pattern with instance fields (sender, args, guild at lines 19-21) and makes blocking database calls on the main thread (line 31). For consistency and to fully address the performance issues mentioned in the PR, GuildCommandS should receive the same refactoring treatment as GuildCommand.

Copilot uses AI. Check for mistakes.
import com.mcatk.guildmanager.GuildItem;
import com.mcatk.guildmanager.Msg;
import com.mcatk.guildmanager.exceptions.ParaLengthException;
Expand Down
Loading
Loading