Skip to content

Commit f6f897b

Browse files
Copilotdmccoystephenson
Copilot
and
dmccoystephenson
committed
Fix config not appearing for existing servers; refactor async-safety; update tests
- Always call saveMissingConfigDefaultsIfNotPresent() so new config keys (Discord webhook settings) are written to existing config.yml files - Refactor DiscordWebhookService: separate config reading (main thread) from HTTP sending (async task) via getWebhookUrl(), prepareJoinMessage(), prepareQuitMessage(), sendWebhookMessage(url, content) - Update JoinHandler/QuitHandler to snapshot all config on main thread - Use MockitoAnnotations.openMocks() instead of deprecated initMocks() - Tests updated for new decomposed API Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
1 parent fa9f28b commit f6f897b

File tree

5 files changed

+108
-94
lines changed

5 files changed

+108
-94
lines changed

‎src/main/java/dansplugins/activitytracker/ActivityTracker.java‎

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package dansplugins.activitytracker;
22

3-
import java.io.File;
43
import java.util.ArrayList;
54
import java.util.Arrays;
65

@@ -124,22 +123,7 @@ public boolean isDebugEnabled() {
124123
}
125124

126125
private void initializeConfig() {
127-
if (configFileExists()) {
128-
performCompatibilityChecks();
129-
}
130-
else {
131-
configService.saveMissingConfigDefaultsIfNotPresent();
132-
}
133-
}
134-
135-
private boolean configFileExists() {
136-
return new File("./plugins/" + getName() + "/config.yml").exists();
137-
}
138-
139-
private void performCompatibilityChecks() {
140-
if (isVersionMismatched()) {
141-
configService.saveMissingConfigDefaultsIfNotPresent();
142-
}
126+
configService.saveMissingConfigDefaultsIfNotPresent();
143127
}
144128

145129
/**

‎src/main/java/dansplugins/activitytracker/eventhandlers/JoinHandler.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,15 @@ private void sendDiscordJoinNotification(Player player) {
6060
if (discordWebhookService.isStaffOnly() && !player.hasPermission(STAFF_PERMISSION)) {
6161
return;
6262
}
63-
final String playerName = player.getName();
63+
final String webhookUrl = discordWebhookService.getWebhookUrl();
64+
final String message = discordWebhookService.prepareJoinMessage(player.getName());
65+
if (webhookUrl == null || message == null) {
66+
return;
67+
}
6468
plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() {
6569
@Override
6670
public void run() {
67-
discordWebhookService.sendJoinNotification(playerName);
71+
discordWebhookService.sendWebhookMessage(webhookUrl, message);
6872
}
6973
});
7074
}

‎src/main/java/dansplugins/activitytracker/eventhandlers/QuitHandler.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,15 @@ private void sendDiscordQuitNotification(Player player) {
7575
if (discordWebhookService.isStaffOnly() && !player.hasPermission(STAFF_PERMISSION)) {
7676
return;
7777
}
78-
final String playerName = player.getName();
78+
final String webhookUrl = discordWebhookService.getWebhookUrl();
79+
final String message = discordWebhookService.prepareQuitMessage(player.getName());
80+
if (webhookUrl == null || message == null) {
81+
return;
82+
}
7983
plugin.getServer().getScheduler().runTaskAsynchronously(plugin, new Runnable() {
8084
@Override
8185
public void run() {
82-
discordWebhookService.sendQuitNotification(playerName);
86+
discordWebhookService.sendWebhookMessage(webhookUrl, message);
8387
}
8488
});
8589
}

‎src/main/java/dansplugins/activitytracker/services/DiscordWebhookService.java‎

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public DiscordWebhookService(ConfigService configService, Logger logger) {
2424

2525
/**
2626
* Checks if the Discord webhook feature is enabled and configured.
27+
* Must be called from the main server thread.
2728
* @return true if enabled and a webhook URL is set.
2829
*/
2930
public boolean isEnabled() {
@@ -40,57 +41,70 @@ public boolean isEnabled() {
4041

4142
/**
4243
* Checks if webhooks should only fire for staff members.
44+
* Must be called from the main server thread.
4345
* @return true if staff-only mode is active.
4446
*/
4547
public boolean isStaffOnly() {
4648
return configService.getBoolean("discordWebhookStaffOnly");
4749
}
4850

4951
/**
50-
* Sends a player join notification to the configured Discord webhook.
51-
* @param playerName The name of the player who joined.
52+
* Returns the configured webhook URL, trimmed.
53+
* Must be called from the main server thread.
54+
* @return the trimmed webhook URL, or null if not configured.
5255
*/
53-
public void sendJoinNotification(String playerName) {
54-
if (!isEnabled()) {
55-
return;
56+
public String getWebhookUrl() {
57+
String url = configService.getString("discordWebhookUrl");
58+
if (url == null) {
59+
return null;
5660
}
61+
String trimmed = url.trim();
62+
return trimmed.isEmpty() ? null : trimmed;
63+
}
64+
65+
/**
66+
* Prepares a join notification message by applying the player name to the configured template.
67+
* Must be called from the main server thread.
68+
* @param playerName The name of the player who joined.
69+
* @return The formatted message, or null if the template is empty/null.
70+
*/
71+
public String prepareJoinMessage(String playerName) {
5772
String template = configService.getString("discordWebhookJoinMessage");
5873
if (template == null || template.isEmpty()) {
59-
return;
74+
return null;
6075
}
61-
String message = template.replace("{player}", playerName);
62-
sendWebhookMessage(message);
76+
return template.replace("{player}", playerName);
6377
}
6478

6579
/**
66-
* Sends a player quit notification to the configured Discord webhook.
80+
* Prepares a quit notification message by applying the player name to the configured template.
81+
* Must be called from the main server thread.
6782
* @param playerName The name of the player who quit.
83+
* @return The formatted message, or null if the template is empty/null.
6884
*/
69-
public void sendQuitNotification(String playerName) {
70-
if (!isEnabled()) {
71-
return;
72-
}
85+
public String prepareQuitMessage(String playerName) {
7386
String template = configService.getString("discordWebhookQuitMessage");
7487
if (template == null || template.isEmpty()) {
75-
return;
88+
return null;
7689
}
77-
String message = template.replace("{player}", playerName);
78-
sendWebhookMessage(message);
90+
return template.replace("{player}", playerName);
7991
}
8092

8193
/**
82-
* Sends a message to the configured Discord webhook URL.
94+
* Sends a message to the specified Discord webhook URL.
95+
* This method performs HTTP I/O and should be called from an async task.
96+
* Does not access Bukkit APIs.
97+
* @param webhookUrl The Discord webhook URL to post to.
8398
* @param content The message content to send.
8499
*/
85-
private void sendWebhookMessage(String content) {
86-
String webhookUrl = configService.getString("discordWebhookUrl");
87-
if (webhookUrl == null || webhookUrl.trim().isEmpty()) {
100+
public void sendWebhookMessage(String webhookUrl, String content) {
101+
if (webhookUrl == null || webhookUrl.isEmpty()) {
88102
return;
89103
}
90104

91105
HttpURLConnection connection = null;
92106
try {
93-
URL url = new URL(webhookUrl.trim());
107+
URL url = new URL(webhookUrl);
94108
connection = (HttpURLConnection) url.openConnection();
95109
connection.setRequestMethod("POST");
96110
connection.setRequestProperty("Content-Type", "application/json");

‎src/test/java/dansplugins/activitytracker/services/DiscordWebhookServiceTest.java‎

Lines changed: 60 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package dansplugins.activitytracker.services;
22

33
import dansplugins.activitytracker.utils.Logger;
4+
import org.junit.After;
45
import org.junit.Before;
56
import org.junit.Test;
67
import org.mockito.Mock;
@@ -21,13 +22,19 @@ public class DiscordWebhookServiceTest {
2122
private Logger logger;
2223

2324
private DiscordWebhookService discordWebhookService;
25+
private AutoCloseable mocks;
2426

2527
@Before
2628
public void setUp() {
27-
MockitoAnnotations.initMocks(this);
29+
mocks = MockitoAnnotations.openMocks(this);
2830
discordWebhookService = new DiscordWebhookService(configService, logger);
2931
}
3032

33+
@After
34+
public void tearDown() throws Exception {
35+
mocks.close();
36+
}
37+
3138
@Test
3239
public void testIsEnabledReturnsFalseWhenDisabled() {
3340
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(false);
@@ -77,93 +84,94 @@ public void testIsStaffOnlyReturnsConfigValue() {
7784
}
7885

7986
@Test
80-
public void testSendJoinNotificationSkipsWhenDisabled() {
81-
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(false);
87+
public void testGetWebhookUrlReturnsTrimmedUrl() {
88+
when(configService.getString("discordWebhookUrl")).thenReturn(" https://discord.com/api/webhooks/test ");
8289

83-
discordWebhookService.sendJoinNotification("TestPlayer");
90+
assertEquals("https://discord.com/api/webhooks/test", discordWebhookService.getWebhookUrl());
91+
}
8492

85-
// Should not attempt to read join message template when disabled
86-
verify(configService, never()).getString("discordWebhookJoinMessage");
93+
@Test
94+
public void testGetWebhookUrlReturnsNullWhenEmpty() {
95+
when(configService.getString("discordWebhookUrl")).thenReturn("");
96+
97+
assertNull(discordWebhookService.getWebhookUrl());
8798
}
8899

89100
@Test
90-
public void testSendQuitNotificationSkipsWhenDisabled() {
91-
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(false);
101+
public void testGetWebhookUrlReturnsNullWhenNull() {
102+
when(configService.getString("discordWebhookUrl")).thenReturn(null);
103+
104+
assertNull(discordWebhookService.getWebhookUrl());
105+
}
92106

93-
discordWebhookService.sendQuitNotification("TestPlayer");
107+
@Test
108+
public void testGetWebhookUrlReturnsNullWhenWhitespaceOnly() {
109+
when(configService.getString("discordWebhookUrl")).thenReturn(" ");
94110

95-
// Should not attempt to read quit message template when disabled
96-
verify(configService, never()).getString("discordWebhookQuitMessage");
111+
assertNull(discordWebhookService.getWebhookUrl());
97112
}
98113

99114
@Test
100-
public void testSendJoinNotificationSkipsWhenTemplateIsEmpty() {
101-
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(true);
102-
when(configService.getString("discordWebhookUrl")).thenReturn("https://discord.com/api/webhooks/test");
103-
when(configService.getString("discordWebhookJoinMessage")).thenReturn("");
115+
public void testPrepareJoinMessageReplacesPlayerName() {
116+
when(configService.getString("discordWebhookJoinMessage")).thenReturn("**{player}** joined!");
104117

105-
discordWebhookService.sendJoinNotification("TestPlayer");
118+
assertEquals("**TestPlayer** joined!", discordWebhookService.prepareJoinMessage("TestPlayer"));
119+
}
106120

107-
// Should check the template but not attempt to fetch URL for sending
108-
verify(configService).getString("discordWebhookJoinMessage");
109-
verify(configService, times(1)).getString("discordWebhookUrl");
121+
@Test
122+
public void testPrepareJoinMessageReturnsNullWhenTemplateIsEmpty() {
123+
when(configService.getString("discordWebhookJoinMessage")).thenReturn("");
124+
125+
assertNull(discordWebhookService.prepareJoinMessage("TestPlayer"));
110126
}
111127

112128
@Test
113-
public void testSendJoinNotificationSkipsWhenTemplateIsNull() {
114-
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(true);
115-
when(configService.getString("discordWebhookUrl")).thenReturn("https://discord.com/api/webhooks/test");
129+
public void testPrepareJoinMessageReturnsNullWhenTemplateIsNull() {
116130
when(configService.getString("discordWebhookJoinMessage")).thenReturn(null);
117131

118-
discordWebhookService.sendJoinNotification("TestPlayer");
132+
assertNull(discordWebhookService.prepareJoinMessage("TestPlayer"));
133+
}
134+
135+
@Test
136+
public void testPrepareQuitMessageReplacesPlayerName() {
137+
when(configService.getString("discordWebhookQuitMessage")).thenReturn("**{player}** left.");
119138

120-
verify(configService).getString("discordWebhookJoinMessage");
121-
verify(configService, times(1)).getString("discordWebhookUrl");
139+
assertEquals("**TestPlayer** left.", discordWebhookService.prepareQuitMessage("TestPlayer"));
122140
}
123141

124142
@Test
125-
public void testSendQuitNotificationSkipsWhenTemplateIsEmpty() {
126-
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(true);
127-
when(configService.getString("discordWebhookUrl")).thenReturn("https://discord.com/api/webhooks/test");
143+
public void testPrepareQuitMessageReturnsNullWhenTemplateIsEmpty() {
128144
when(configService.getString("discordWebhookQuitMessage")).thenReturn("");
129145

130-
discordWebhookService.sendQuitNotification("TestPlayer");
131-
132-
verify(configService).getString("discordWebhookQuitMessage");
133-
verify(configService, times(1)).getString("discordWebhookUrl");
146+
assertNull(discordWebhookService.prepareQuitMessage("TestPlayer"));
134147
}
135148

136149
@Test
137-
public void testSendQuitNotificationSkipsWhenTemplateIsNull() {
138-
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(true);
139-
when(configService.getString("discordWebhookUrl")).thenReturn("https://discord.com/api/webhooks/test");
150+
public void testPrepareQuitMessageReturnsNullWhenTemplateIsNull() {
140151
when(configService.getString("discordWebhookQuitMessage")).thenReturn(null);
141152

142-
discordWebhookService.sendQuitNotification("TestPlayer");
143-
144-
verify(configService).getString("discordWebhookQuitMessage");
145-
verify(configService, times(1)).getString("discordWebhookUrl");
153+
assertNull(discordWebhookService.prepareQuitMessage("TestPlayer"));
146154
}
147155

148156
@Test
149-
public void testSendJoinNotificationHandlesInvalidUrl() {
150-
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(true);
151-
when(configService.getString("discordWebhookUrl")).thenReturn("not-a-valid-url");
152-
when(configService.getString("discordWebhookJoinMessage")).thenReturn("**{player}** joined!");
153-
154-
discordWebhookService.sendJoinNotification("TestPlayer");
157+
public void testSendWebhookMessageSkipsWhenUrlIsNull() {
158+
discordWebhookService.sendWebhookMessage(null, "test message");
155159

156-
// Error should be logged gracefully
157-
verify(logger).log(contains("Failed to send Discord webhook message"));
160+
// No HTTP call should be attempted, no errors logged
161+
verifyNoInteractions(logger);
158162
}
159163

160164
@Test
161-
public void testSendQuitNotificationHandlesInvalidUrl() {
162-
when(configService.getBoolean("discordWebhookEnabled")).thenReturn(true);
163-
when(configService.getString("discordWebhookUrl")).thenReturn("not-a-valid-url");
164-
when(configService.getString("discordWebhookQuitMessage")).thenReturn("**{player}** left.");
165+
public void testSendWebhookMessageSkipsWhenUrlIsEmpty() {
166+
discordWebhookService.sendWebhookMessage("", "test message");
165167

166-
discordWebhookService.sendQuitNotification("TestPlayer");
168+
// No HTTP call should be attempted, no errors logged
169+
verifyNoInteractions(logger);
170+
}
171+
172+
@Test
173+
public void testSendWebhookMessageHandlesInvalidUrl() {
174+
discordWebhookService.sendWebhookMessage("not-a-valid-url", "**TestPlayer** joined!");
167175

168176
// Error should be logged gracefully
169177
verify(logger).log(contains("Failed to send Discord webhook message"));

0 commit comments

Comments
 (0)