-
Notifications
You must be signed in to change notification settings - Fork 0
Add Discord webhook notifications for player join/quit events #77
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.
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
d06aed3
9a60956
877151a
fa9f28b
f6f897b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,10 +2,12 @@ | |||||||||||||||||
|
|
||||||||||||||||||
| import dansplugins.activitytracker.data.PersistentData; | ||||||||||||||||||
| import dansplugins.activitytracker.factories.SessionFactory; | ||||||||||||||||||
| import dansplugins.activitytracker.services.DiscordWebhookService; | ||||||||||||||||||
| import org.bukkit.entity.Player; | ||||||||||||||||||
| import org.bukkit.event.EventHandler; | ||||||||||||||||||
| import org.bukkit.event.Listener; | ||||||||||||||||||
| import org.bukkit.event.player.PlayerJoinEvent; | ||||||||||||||||||
| import org.bukkit.plugin.java.JavaPlugin; | ||||||||||||||||||
|
|
||||||||||||||||||
| import dansplugins.activitytracker.objects.ActivityRecord; | ||||||||||||||||||
| import dansplugins.activitytracker.objects.Session; | ||||||||||||||||||
|
|
@@ -18,11 +20,17 @@ public class JoinHandler implements Listener { | |||||||||||||||||
| private final ActivityRecordService activityRecordService; | ||||||||||||||||||
| private final PersistentData persistentData; | ||||||||||||||||||
| private final SessionFactory sessionFactory; | ||||||||||||||||||
| private final DiscordWebhookService discordWebhookService; | ||||||||||||||||||
| private final JavaPlugin plugin; | ||||||||||||||||||
|
|
||||||||||||||||||
| public JoinHandler(ActivityRecordService activityRecordService, PersistentData persistentData, SessionFactory sessionFactory) { | ||||||||||||||||||
| private static final String STAFF_PERMISSION = "at.staff"; | ||||||||||||||||||
|
|
||||||||||||||||||
| public JoinHandler(ActivityRecordService activityRecordService, PersistentData persistentData, SessionFactory sessionFactory, DiscordWebhookService discordWebhookService, JavaPlugin plugin) { | ||||||||||||||||||
| this.activityRecordService = activityRecordService; | ||||||||||||||||||
| this.persistentData = persistentData; | ||||||||||||||||||
| this.sessionFactory = sessionFactory; | ||||||||||||||||||
| this.discordWebhookService = discordWebhookService; | ||||||||||||||||||
| this.plugin = plugin; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @EventHandler() | ||||||||||||||||||
|
|
@@ -41,5 +49,22 @@ public void handle(PlayerJoinEvent event) { | |||||||||||||||||
| record.addSession(newSession); | ||||||||||||||||||
| record.setMostRecentSession(newSession); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| sendDiscordJoinNotification(player); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private void sendDiscordJoinNotification(Player player) { | ||||||||||||||||||
| if (!discordWebhookService.isEnabled()) { | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
| if (discordWebhookService.isStaffOnly() && !player.hasPermission(STAFF_PERMISSION)) { | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
| plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() { | ||||||||||||||||||
| @Override | ||||||||||||||||||
| public void run() { | ||||||||||||||||||
| discordWebhookService.sendJoinNotification(player.getName()); | ||||||||||||||||||
| } | ||||||||||||||||||
| }); | ||||||||||||||||||
|
Comment on lines
+68
to
+73
|
||||||||||||||||||
| plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() { | |
| @Override | |
| public void run() { | |
| discordWebhookService.sendJoinNotification(playerName); | |
| } | |
| }); | |
| // Call sendJoinNotification on the main thread to avoid async Bukkit API access | |
| discordWebhookService.sendJoinNotification(playerName); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,11 +1,13 @@ | ||||||||||||||||||||
| package dansplugins.activitytracker.eventhandlers; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import dansplugins.activitytracker.exceptions.NoSessionException; | ||||||||||||||||||||
| import dansplugins.activitytracker.services.DiscordWebhookService; | ||||||||||||||||||||
| import dansplugins.activitytracker.utils.Logger; | ||||||||||||||||||||
| import org.bukkit.entity.Player; | ||||||||||||||||||||
| import org.bukkit.event.EventHandler; | ||||||||||||||||||||
| import org.bukkit.event.Listener; | ||||||||||||||||||||
| import org.bukkit.event.player.PlayerQuitEvent; | ||||||||||||||||||||
| import org.bukkit.plugin.java.JavaPlugin; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import dansplugins.activitytracker.data.PersistentData; | ||||||||||||||||||||
| import dansplugins.activitytracker.objects.ActivityRecord; | ||||||||||||||||||||
|
|
@@ -17,10 +19,16 @@ | |||||||||||||||||||
| public class QuitHandler implements Listener { | ||||||||||||||||||||
| private final PersistentData persistentData; | ||||||||||||||||||||
| private final Logger logger; | ||||||||||||||||||||
| private final DiscordWebhookService discordWebhookService; | ||||||||||||||||||||
| private final JavaPlugin plugin; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public QuitHandler(PersistentData persistentData, Logger logger) { | ||||||||||||||||||||
| private static final String STAFF_PERMISSION = "at.staff"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public QuitHandler(PersistentData persistentData, Logger logger, DiscordWebhookService discordWebhookService, JavaPlugin plugin) { | ||||||||||||||||||||
| this.persistentData = persistentData; | ||||||||||||||||||||
| this.logger = logger; | ||||||||||||||||||||
| this.discordWebhookService = discordWebhookService; | ||||||||||||||||||||
| this.plugin = plugin; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @EventHandler() | ||||||||||||||||||||
|
|
@@ -56,5 +64,22 @@ public void handle(PlayerQuitEvent event) { | |||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||
| logger.log("ERROR: Failed to properly end session for " + player.getName() + ": " + e.getMessage()); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| sendDiscordQuitNotification(player); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private void sendDiscordQuitNotification(Player player) { | ||||||||||||||||||||
| if (!discordWebhookService.isEnabled()) { | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if (discordWebhookService.isStaffOnly() && !player.hasPermission(STAFF_PERMISSION)) { | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() { | ||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public void run() { | ||||||||||||||||||||
| discordWebhookService.sendQuitNotification(player.getName()); | ||||||||||||||||||||
|
||||||||||||||||||||
| plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() { | |
| @Override | |
| public void run() { | |
| discordWebhookService.sendQuitNotification(player.getName()); | |
| final String playerName = player.getName(); | |
| plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() { | |
| @Override | |
| public void run() { | |
| discordWebhookService.sendQuitNotification(playerName); |
Copilot
AI
Mar 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same async-safety concern as in JoinHandler: the runnable calls discordWebhookService.sendQuitNotification(...), which reads plugin config (and may invoke Logger.log() which also reads config). This introduces Bukkit API calls from an asynchronous thread. Snapshot the required config/debug values on the main thread before scheduling, and keep the async work limited to the HTTP request.
| plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() { | |
| @Override | |
| public void run() { | |
| discordWebhookService.sendQuitNotification(playerName); | |
| } | |
| }); | |
| // Call on the main thread to avoid accessing Bukkit APIs from an async task | |
| discordWebhookService.sendQuitNotification(playerName); |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,147 @@ | ||||||||||||||
| package dansplugins.activitytracker.services; | ||||||||||||||
|
|
||||||||||||||
| import java.io.IOException; | ||||||||||||||
| import java.io.OutputStream; | ||||||||||||||
| import java.net.HttpURLConnection; | ||||||||||||||
| import java.net.URL; | ||||||||||||||
| import java.nio.charset.StandardCharsets; | ||||||||||||||
|
|
||||||||||||||
| import dansplugins.activitytracker.utils.Logger; | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Service for sending Discord webhook notifications when players join or leave the server. | ||||||||||||||
| * @author Daniel McCoy Stephenson | ||||||||||||||
| */ | ||||||||||||||
| public class DiscordWebhookService { | ||||||||||||||
| private final ConfigService configService; | ||||||||||||||
| private final Logger logger; | ||||||||||||||
|
|
||||||||||||||
| public DiscordWebhookService(ConfigService configService, Logger logger) { | ||||||||||||||
| this.configService = configService; | ||||||||||||||
| this.logger = logger; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Checks if the Discord webhook feature is enabled and configured. | ||||||||||||||
| * @return true if enabled and a webhook URL is set. | ||||||||||||||
| */ | ||||||||||||||
| public boolean isEnabled() { | ||||||||||||||
| if (!configService.getBoolean("discordWebhookEnabled")) { | ||||||||||||||
| return false; | ||||||||||||||
| } | ||||||||||||||
| String url = configService.getString("discordWebhookUrl"); | ||||||||||||||
| return url != null && !url.isEmpty(); | ||||||||||||||
|
||||||||||||||
| return url != null && !url.isEmpty(); | |
| if (url == null) { | |
| return false; | |
| } | |
| String trimmed = url.trim(); | |
| return !trimmed.isEmpty(); |
Outdated
Copilot
AI
Mar 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sendJoinNotification builds and sends a webhook payload regardless of the discordWebhookEnabled flag. Since the service already exposes isEnabled(), these public send methods should short-circuit when disabled (or have sendWebhookMessage enforce the enabled flag) to ensure the config toggle is always respected, even if other call sites are added later.
Copilot
AI
Mar 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HttpURLConnection isn't disconnected and the response stream is never consumed/closed. On some JVMs this can leak sockets/file descriptors and degrade reliability over time. Ensure the response InputStream/ErrorStream is closed (even if you only need the status code) and call connection.disconnect() in a finally block (try-with-resources for the output stream also helps).
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,4 +21,6 @@ permissions: | |
| at.config: | ||
| default: op | ||
| at.list: | ||
| default: op | ||
|
Comment on lines
23
to
+24
|
||
| at.staff: | ||
| default: op | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
player.getName()is called inside the async task. Bukkit/Spigot API objects (includingPlayer) are generally not thread-safe, and on quit the player object can become invalid. Capture the player name (and any other Bukkit-derived values) on the main thread before scheduling, and only pass the plainStringinto the async runnable.