Skip to content

Commit f0b0948

Browse files
BenCodezBenCodez
committed
Keep timed permission expiry on platform threads
1 parent 9b8c064 commit f0b0948

3 files changed

Lines changed: 125 additions & 15 deletions

File tree

‎AdvancedCore/src/main/java/com/bencodez/advancedcore/api/permissions/PermissionHandler.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,39 @@ public PermissionHandler(AdvancedCorePlugin plugin) {
8888
}
8989
}
9090

91+
92+
void scheduleExpiration(PlayerPermissionHandler handle, String permission, long expectedExpireAt, long delayMillis) {
93+
try {
94+
timer.schedule(() -> dispatchExpiration(handle, permission, expectedExpireAt),
95+
Math.max(0L, delayMillis), java.util.concurrent.TimeUnit.MILLISECONDS);
96+
} catch (RuntimeException failure) {
97+
plugin.debug(failure);
98+
throw failure;
99+
}
100+
}
101+
102+
private void dispatchExpiration(PlayerPermissionHandler handle, String permission, long expectedExpireAt) {
103+
if (!handle.isExpirationCurrent(permission, expectedExpireAt)) return;
104+
try {
105+
plugin.getBukkitScheduler().runTask(plugin, () -> {
106+
if (!handle.isExpirationCurrent(permission, expectedExpireAt)) return;
107+
Player player = Bukkit.getPlayer(handle.getUuid());
108+
if (player == null) {
109+
handle.expirePermission(permission, expectedExpireAt, false);
110+
return;
111+
}
112+
try {
113+
plugin.getBukkitScheduler().runTask(plugin,
114+
() -> handle.expirePermission(permission, expectedExpireAt, true), player);
115+
} catch (RuntimeException failure) {
116+
plugin.debug(failure);
117+
}
118+
});
119+
} catch (RuntimeException failure) {
120+
plugin.debug(failure);
121+
}
122+
}
123+
91124
public void addPermission(Player player, String permission) {
92125
addPermission(player.getUniqueId(), permission);
93126
}
@@ -276,6 +309,7 @@ public void shutDown() {
276309
saveTimedPerms(perms);
277310
saveTimedPerms(permsToAdd);
278311
plugin.getServerDataFile().saveData();
312+
timer.shutdownNow();
279313
}
280314

281315
private void saveTimedPerms(ConcurrentHashMap<UUID, PlayerPermissionHandler> map) {

‎AdvancedCore/src/main/java/com/bencodez/advancedcore/api/permissions/PlayerPermissionHandler.java‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import java.util.Map.Entry;
66
import java.util.Set;
77
import java.util.UUID;
8-
import java.util.concurrent.TimeUnit;
98

109
import org.bukkit.entity.Player;
1110
import org.bukkit.permissions.PermissionAttachment;
@@ -70,7 +69,7 @@ public PlayerPermissionHandler(UUID uuid, PermissionAttachment attachment, Permi
7069
* @param duration How long the permission should last
7170
* @return this
7271
*/
73-
public PlayerPermissionHandler addExpiration(String perm, ParsedDuration duration) {
72+
public synchronized PlayerPermissionHandler addExpiration(String perm, ParsedDuration duration) {
7473
if (duration == null || duration.isEmpty()) {
7574
return addPerm(perm);
7675
}
@@ -87,7 +86,7 @@ public PlayerPermissionHandler addExpiration(String perm, ParsedDuration duratio
8786
}
8887

8988
long delayMillis = duration.delayMillisFromNow();
90-
handler.getTimer().schedule(() -> removePermission(perm), delayMillis, TimeUnit.MILLISECONDS);
89+
handler.scheduleExpiration(this, perm, expireAt, delayMillis);
9190

9291
return this;
9392
}
@@ -106,7 +105,7 @@ public PlayerPermissionHandler addExpiration(String perm, long seconds) {
106105
* @param duration Duration; empty means permanent
107106
* @return this
108107
*/
109-
public PlayerPermissionHandler addOfflinePerm(String perm, ParsedDuration duration) {
108+
public synchronized PlayerPermissionHandler addOfflinePerm(String perm, ParsedDuration duration) {
110109
if (permsToAdd == null) {
111110
permsToAdd = new HashMap<>();
112111
}
@@ -128,7 +127,7 @@ public PlayerPermissionHandler addOfflinePerm(String perm, long seconds) {
128127
* @param perm Permission node
129128
* @return this
130129
*/
131-
public PlayerPermissionHandler addPerm(String perm) {
130+
public synchronized PlayerPermissionHandler addPerm(String perm) {
132131
persistentPermissions.add(perm);
133132

134133
if (attachment != null) {
@@ -141,7 +140,7 @@ public PlayerPermissionHandler addPerm(String perm) {
141140
/**
142141
* Re-applies stored permissions after an attachment is created on login.
143142
*/
144-
public void onLogin(Player player) {
143+
public synchronized void onLogin(Player player) {
145144
if (attachment == null) {
146145
return;
147146
}
@@ -184,10 +183,43 @@ public void onLogout(Player player) {
184183
// intentionally empty
185184
}
186185

186+
187+
/** True only while this exact scheduled expiration still owns the timed grant. */
188+
synchronized boolean isExpirationCurrent(String perm, long expectedExpireAt) {
189+
if (timedPermissions == null) return false;
190+
Long current = timedPermissions.get(perm);
191+
return current != null && current.longValue() == expectedExpireAt;
192+
}
193+
194+
/**
195+
* Expire only the scheduled grant that still owns this permission. The
196+
* attachment flag is true only from the player's owning platform scheduler.
197+
*/
198+
synchronized void expirePermission(String perm, long expectedExpireAt, boolean updateAttachment) {
199+
if (timedPermissions == null) return;
200+
Long current = timedPermissions.get(perm);
201+
if (current == null || current.longValue() != expectedExpireAt) return;
202+
if (!timedPermissions.remove(perm, current)) return;
203+
persistentPermissions.remove(perm);
204+
if (permsToAdd != null) permsToAdd.remove(perm);
205+
if (updateAttachment && attachment != null) {
206+
attachment.setPermission(perm, false);
207+
attachment.getPermissions().remove(perm);
208+
}
209+
removeHandlerIfEmpty();
210+
}
211+
212+
private void removeHandlerIfEmpty() {
213+
boolean noTracked = persistentPermissions.isEmpty()
214+
&& (timedPermissions == null || timedPermissions.isEmpty())
215+
&& (permsToAdd == null || permsToAdd.isEmpty());
216+
if (noTracked && (attachment == null || attachment.getPermissions().isEmpty())) handler.removePermission(uuid);
217+
}
218+
187219
/**
188220
* Removes a permission from both internal tracking and the live attachment (if online).
189221
*/
190-
public void removePermission(String perm) {
222+
public synchronized void removePermission(String perm) {
191223
persistentPermissions.remove(perm);
192224
if (timedPermissions != null) {
193225
timedPermissions.remove(perm);
@@ -201,13 +233,6 @@ public void removePermission(String perm) {
201233
attachment.getPermissions().remove(perm);
202234
}
203235

204-
// If nothing tracked and attachment has nothing, drop handler state entirely
205-
boolean noTracked = persistentPermissions.isEmpty()
206-
&& (timedPermissions == null || timedPermissions.isEmpty())
207-
&& (permsToAdd == null || permsToAdd.isEmpty());
208-
209-
if (noTracked && attachment != null && attachment.getPermissions().isEmpty()) {
210-
handler.removePermission(uuid);
211-
}
236+
removeHandlerIfEmpty();
212237
}
213238
}

‎AdvancedCore/src/test/java/com/bencodez/advancedcore/api/permissions/PlayerPermissionHandlerThreadingTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package com.bencodez.advancedcore.api.permissions;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
import static org.mockito.ArgumentMatchers.*;
5+
import static org.mockito.Mockito.*;
6+
7+
import java.util.UUID;
8+
9+
import org.bukkit.permissions.PermissionAttachment;
10+
import org.junit.jupiter.api.Test;
11+
12+
import com.bencodez.simpleapi.time.ParsedDuration;
13+
14+
class PlayerPermissionHandlerThreadingTest {
15+
16+
@Test
17+
void staleExpirationCannotRevokeExtendedPermission() {
18+
PermissionHandler manager = mock(PermissionHandler.class);
19+
PermissionAttachment attachment = mock(PermissionAttachment.class);
20+
when(attachment.getPermissions()).thenReturn(new java.util.LinkedHashMap<>());
21+
PlayerPermissionHandler handler = new PlayerPermissionHandler(UUID.randomUUID(), attachment, manager);
22+
23+
handler.addExpiration("example.use", ParsedDuration.ofMillis(60_000));
24+
long firstExpiry = handler.getTimedPermissions().get("example.use");
25+
handler.addExpiration("example.use", ParsedDuration.ofMillis(120_000));
26+
long extendedExpiry = handler.getTimedPermissions().get("example.use");
27+
assertTrue(extendedExpiry >= firstExpiry);
28+
clearInvocations(attachment);
29+
30+
handler.expirePermission("example.use", firstExpiry, true);
31+
32+
assertEquals(extendedExpiry, handler.getTimedPermissions().get("example.use"));
33+
verify(attachment, never()).setPermission("example.use", false);
34+
}
35+
36+
@Test
37+
void offOwnerExpirationUpdatesStateWithoutTouchingAttachment() {
38+
PermissionHandler manager = mock(PermissionHandler.class);
39+
PermissionAttachment attachment = mock(PermissionAttachment.class);
40+
when(attachment.getPermissions()).thenReturn(new java.util.LinkedHashMap<>());
41+
PlayerPermissionHandler handler = new PlayerPermissionHandler(UUID.randomUUID(), attachment, manager);
42+
43+
handler.addExpiration("example.use", ParsedDuration.ofMillis(60_000));
44+
long expiry = handler.getTimedPermissions().get("example.use");
45+
clearInvocations(attachment);
46+
handler.expirePermission("example.use", expiry, false);
47+
48+
assertFalse(handler.getTimedPermissions().containsKey("example.use"));
49+
verify(attachment, never()).setPermission("example.use", false);
50+
}
51+
}

0 commit comments

Comments
 (0)