The same registration as {@link #addMember(OfflinePlayer, GuildHandler)}: a player is only + * in the guild once, a stale invite is dropped, and the member carries the date they joined. + * The role is whatever the caller decided, which for a code is the lowest role in the guild. + * + * @param guildMember the member to add */ public void addMemberByCode(GuildMember guildMember) { - members.add(guildMember); + addMember(guildMember); + } + + /** + * Registers a member with the guild. + * + *
A member is identified by their uuid, not by the object: {@link GuildMember} does not + * override {@code equals}, so comparing objects would never find the member that is already in + * the guild and every caller would be free to add a second entry for the same player. + * + * @param member the member to register + */ + private void addMember(GuildMember member) { + if (getMember(member.getUuid()) != null) return; + removeInvitedMember(member.getUuid()); + members.add(member); + if (member.getJoinDate() == 0L) { + member.setJoinDate(System.currentTimeMillis()); + } } /** diff --git a/src/main/java/me/glaremasters/guilds/guild/GuildHandler.java b/src/main/java/me/glaremasters/guilds/guild/GuildHandler.java index eaa16e32e..cdf38a89c 100644 --- a/src/main/java/me/glaremasters/guilds/guild/GuildHandler.java +++ b/src/main/java/me/glaremasters/guilds/guild/GuildHandler.java @@ -6,6 +6,7 @@ import co.aikar.commands.ACFUtil; import co.aikar.commands.PaperCommandManager; import me.glaremasters.guilds.Guilds; +import me.glaremasters.guilds.api.events.GuildJoinEvent; import me.glaremasters.guilds.configuration.sections.GuildSettings; import me.glaremasters.guilds.configuration.sections.GuildVaultSettings; import me.glaremasters.guilds.configuration.sections.PluginSettings; @@ -1107,6 +1108,11 @@ public void removeRolePermsFromAll(final Permission permission, final Guild guil /** * Handle inviting player when redeeming a code * + *
A code is a way into a guild, so this does what {@code /guild accept} does: it fires the + * cancellable {@link GuildJoinEvent} before anything is consumed, registers the member the + * same way an invite does, and hands the member the permissions their guild and role grant. + * A plugin that cancels the join event stops the redeem, and the code keeps its use. + * * @param manager command manager * @param player the player redeeming the code * @param guild the guild they are trying to join @@ -1116,11 +1122,24 @@ public void handleInvite(PaperCommandManager manager, Player player, Guild guild if (code.getUses() <= 0) ACFUtil.sneaky(new ExpectationNotMet(Messages.CODES__OUT)); + final GuildJoinEvent event = new GuildJoinEvent(player, guild); + Bukkit.getPluginManager().callEvent(event); + + if (event.isCancelled()) { + return; + } + code.addRedeemer(player); - guild.addMemberByCode(new GuildMember(player.getUniqueId(), getLowestGuildRole())); + guild.addMember(player, this); addToMemberCache(player.getUniqueId(), guild.getId()); + final Permission permission = getGuildsPlugin().getPermissions(); + if (permission != null) { + addGuildPerms(permission, player); + addRolePerm(permission, player); + } + if (ClaimUtils.isEnabled(settingsManager)) { WorldGuardWrapper wrapper = WorldGuardWrapper.getInstance(); ClaimUtils.getGuildClaim(wrapper, player, guild).ifPresent(region -> ClaimUtils.addMember(region, player)); diff --git a/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt b/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt index df3c4b570..368f1d9b7 100644 --- a/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt +++ b/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt @@ -15,6 +15,8 @@ import co.aikar.commands.annotation.Syntax import co.aikar.commands.annotation.Values import me.glaremasters.guilds.Guilds import me.glaremasters.guilds.configuration.sections.CodeSettings +import me.glaremasters.guilds.cooldowns.Cooldown +import me.glaremasters.guilds.cooldowns.CooldownHandler import me.glaremasters.guilds.exceptions.ExpectationNotMet import me.glaremasters.guilds.guild.Guild import me.glaremasters.guilds.guild.GuildHandler @@ -32,6 +34,8 @@ internal class CommandCode : BaseCommand() { lateinit var settingsManager: SettingsManager @Dependency lateinit var guildHandler: GuildHandler + @Dependency + lateinit var cooldownHandler: CooldownHandler @Subcommand("code create") @Description("{@@descriptions.code-create}") @@ -98,6 +102,14 @@ internal class CommandCode : BaseCommand() { fun redeem(@Conditions("NoGuild") player: Player, code: String) { val guild = guildHandler.getGuildByCode(code) ?: throw ExpectationNotMet(Messages.CODES__INVALID_CODE) + // A code is a way into a guild, so it is refused while the join cooldown runs, the same + // way `/guild accept` and `/guild create` refuse it. + val cooldown = Cooldown.Type.Join.name + + if (cooldownHandler.hasCooldown(cooldown, player.uniqueId)) { + throw ExpectationNotMet(Messages.ACCEPT__COOLDOWN, "{amount}", cooldownHandler.getRemaining(cooldown, player.uniqueId).toString()) + } + if (guildHandler.checkIfFull(guild)) { throw ExpectationNotMet(Messages.ACCEPT__GUILD_FULL) } diff --git a/src/test/e2e/tests/codes.spec.ts b/src/test/e2e/tests/codes.spec.ts index 2e51faf30..faeaf33d0 100644 --- a/src/test/e2e/tests/codes.spec.ts +++ b/src/test/e2e/tests/codes.spec.ts @@ -1,7 +1,8 @@ import { describe, expect, test } from '@plugwright/runner'; import { commandWithout, expectReceived } from '../plugins/support/assertions.js'; import { MSG } from '../plugins/support/expected.js'; -import { guildList } from '../plugins/support/state.js'; +import { ROLE } from '../plugins/support/fixtures.js'; +import { guildList, guildMembers } from '../plugins/support/state.js'; import { stripColors } from '../plugins/support/text.js'; /** @@ -63,6 +64,65 @@ test('a code can be redeemed and puts the player in the guild', async ({ player, const entry = (await guildList(joiner)).find(item => item.name === guild)!; expect(entry.memberCount).toBe(2); + + // The roster is the one an invited member appears on, with the same role. + const roster = await guildMembers(player); + expect(roster.map(member => member.name).sort()).toEqual([player.username, joiner.username].sort()); + expect(roster.find(member => member.name === joiner.username)!.role).toBe(ROLE.member); +}); + +test('a player who joins by code gets the permissions their guild grants them', async ({ + player, + createPlayer, + guilds, +}) => { + const guild = await guilds.createGuild(player); + const created = player.getMessageBufferIndex(); + await guilds.run(player, '/guild code create 2'); + await expectReceived(player, MSG.codeCreated, created); + const code = parseCode(player.messageBuffer.slice(created).join('\n'))!; + + const joiner = await createPlayer({ username: guilds.playerName() }); + const joining = joiner.getMessageBufferIndex(); + await guilds.run(joiner, `/guild code redeem ${code}`); + await expectReceived(joiner, MSG.codeJoined(guild), joining); + + // tiers.yml grants guilds.command.admin to every member, and the runner denies that node on + // LuckPerms' default group, so an admin command answering at all is the proof that the guild + // handed the node over. LuckPerms applies it to an online player on its own schedule, so the + // probe is re-issued until it works. + const asMember = joiner.getMessageBufferIndex(); + try { + await guilds.awaitEffect(joiner, `/guild admin motd ${guild}`, MSG.motdNotSet); + } catch { + const told = joiner.messageBuffer.slice(asMember).map(stripColors).join(' | '); + throw new Error(`the joiner never got the permissions their guild grants; they were told: ${told}`); + } +}); + +test('a code cannot be redeemed while the join cooldown is running', async ({ player, createPlayer, guilds }) => { + const guild = await guilds.createGuild(player); + const created = player.getMessageBufferIndex(); + await guilds.run(player, '/guild code create 2'); + await expectReceived(player, MSG.codeCreated, created); + const code = parseCode(player.messageBuffer.slice(created).join('\n'))!; + + const joiner = await createPlayer({ username: guilds.playerName() }); + const joining = joiner.getMessageBufferIndex(); + await guilds.run(joiner, `/guild code redeem ${code}`); + await expectReceived(joiner, MSG.codeJoined(guild), joining); + + // Leaving starts the join cooldown, and joining a guild is what that cooldown is about. A code + // is a way into a guild, so it is refused the same way an invite is. + await guilds.run(joiner, '/guild leave'); + await expect(joiner).toHaveReceivedMessage(MSG.leaveWarning); + await guilds.confirm(joiner); + await expect(joiner).toHaveReceivedMessage(MSG.leaveSuccess); + + const blocked = joiner.getMessageBufferIndex(); + await guilds.run(joiner, `/guild code redeem ${code}`); + await expectReceived(joiner, MSG.acceptCooldown, blocked); + await expect(joiner).not.toHaveReceivedMessage(MSG.codeJoined(guild), { since: blocked }); }); test('an invalid code is refused', async ({ player, createPlayer, guilds }) => { diff --git a/src/test/java/me/glaremasters/guilds/guild/GuildCodeTest.java b/src/test/java/me/glaremasters/guilds/guild/GuildCodeTest.java new file mode 100644 index 000000000..2ea257f69 --- /dev/null +++ b/src/test/java/me/glaremasters/guilds/guild/GuildCodeTest.java @@ -0,0 +1,81 @@ +package me.glaremasters.guilds.guild; + +import org.bukkit.entity.Player; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; + +/** + * Covers what one use of an invite code means. + * + *
A code is a counter, a list of the players who spent it, and the player who made it. Those + * three things are the whole of its stored shape, and the code redeem path is not allowed to + * change any of them: a redeem spends exactly one use, records exactly one redeemer, and the + * creator and the remaining count are what the next reader sees. + */ +class GuildCodeTest { + + private static final String ID = "ABC1234"; + + private GuildCode code; + private UUID creatorId; + + @BeforeEach + void setUp() { + creatorId = UUID.randomUUID(); + code = new GuildCode(ID, 3, creatorId, new ArrayList<>()); + } + + @Test + @DisplayName("a redeem spends one use and records one redeemer") + void aRedeemSpendsOneUse() { + final UUID redeemer = UUID.randomUUID(); + + code.addRedeemer(player(redeemer)); + + assertEquals(2, code.getUses()); + assertEquals(Arrays.asList(redeemer), code.getRedeemers()); + } + + @Test + @DisplayName("a redeem leaves the creator and the id alone") + void aRedeemKeepsTheCreator() { + code.addRedeemer(player(UUID.randomUUID())); + + assertEquals(ID, code.getId()); + assertSame(creatorId, code.getCreator()); + } + + @Test + @DisplayName("the same player can spend the code twice, and it costs two uses") + void theSamePlayerCanRedeemTwice() { + // Documented, not endorsed: a player who leaves the guild they joined with a code can + // come back and spend the code again, once the join cooldown has run out. The code + // charges them for it, and the redeemers list names them twice. + final UUID redeemer = UUID.randomUUID(); + final Player player = player(redeemer); + + code.addRedeemer(player); + code.addRedeemer(player); + + assertEquals(1, code.getUses()); + assertEquals(Arrays.asList(redeemer, redeemer), code.getRedeemers()); + } + + /** + * A player with the given uuid. + */ + private static Player player(final UUID uuid) { + final Player player = Mockito.mock(Player.class); + Mockito.when(player.getUniqueId()).thenReturn(uuid); + return player; + } +} diff --git a/src/test/java/me/glaremasters/guilds/guild/GuildHandlerRedeemTest.java b/src/test/java/me/glaremasters/guilds/guild/GuildHandlerRedeemTest.java new file mode 100644 index 000000000..4dedce0d3 --- /dev/null +++ b/src/test/java/me/glaremasters/guilds/guild/GuildHandlerRedeemTest.java @@ -0,0 +1,267 @@ +package me.glaremasters.guilds.guild; + +import ch.jalu.configme.SettingsManager; +import co.aikar.commands.PaperCommandManager; +import me.glaremasters.guilds.Guilds; +import me.glaremasters.guilds.api.events.GuildJoinEvent; +import me.glaremasters.guilds.configuration.sections.HooksSettings; +import me.glaremasters.guilds.configuration.sections.PluginSettings; +import net.milkbowl.vault.permission.Permission; +import org.bukkit.Bukkit; +import org.bukkit.OfflinePlayer; +import org.bukkit.entity.Player; +import org.bukkit.plugin.PluginManager; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Collections; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Covers what {@code /guild code redeem} does to the player who redeems a code. + * + *
{@code /guild accept} and {@code /guild code redeem} are the two ways into a guild, and they + * were not doing the same work. Accept fired the cancellable {@link GuildJoinEvent}, applied the + * guild's tier permissions and the member's role permissions, and registered the member through + * {@link Guild#addMember}; redeeming a code did none of that. A player who joined by code was a + * member in name only: third-party plugins listening for a join never heard about it, a listener + * that cancelled the event could not stop it, and the player kept no permissions until their next + * login. + * + *
Only the bookkeeping is exercised here. The claim and the announcement need a claim plugin and
+ * a real command manager, and the permissions are asserted through the Vault {@link Permission}
+ * mock, which is the same call the plugin makes.
+ */
+class GuildHandlerRedeemTest {
+
+ private static final String CODE = "ABC1234";
+ private static final String TIER_PERM = "guilds.command.admin";
+ private static final String ROLE_NODE = "guilds.roles.member";
+
+ @TempDir
+ File dataFolder;
+
+ private MockedStatic {@code /guild accept} and {@code /guild code redeem} are the two ways into a guild, and they
+ * used to register the member differently. Accept went through {@link Guild#addMember}, which
+ * cleared a stale invite and stamped the member with the date they joined. A code went through
+ * {@link Guild#addMemberByCode}, which did neither, so a member who joined by code showed a join
+ * date of zero — the epoch, in the members list — until their next login patched it up.
+ *
+ * Both now go through the same registration.
+ */
+class GuildMemberRegistrationTest {
+
+ private static final GuildRole MASTER = new GuildRole("GuildMaster", "guilds.roles.master", 0);
+ private static final GuildRole MEMBER = new GuildRole("Member", "guilds.roles.member", 3);
+
+ private Guild guild;
+ private GuildMember master;
+ private UUID playerId;
+
+ @BeforeEach
+ void setUp() {
+ master = new GuildMember(UUID.randomUUID(), MASTER);
+ playerId = UUID.randomUUID();
+
+ guild = new Guild(UUID.randomUUID());
+ guild.setMembers(new ArrayList<>());
+ guild.getMembers().add(master);
+ guild.setInvitedMembers(new ArrayList<>());
+ guild.setGuildMaster(master);
+ }
+
+ @Test
+ @DisplayName("a member added by code is stamped with the date they joined")
+ void codeMemberHasAJoinDate() {
+ guild.addMemberByCode(new GuildMember(playerId, MEMBER));
+
+ final GuildMember member = guild.getMember(playerId);
+ assertNotNull(member);
+ assertTrue(member.getJoinDate() > 0L, "the member should have a join date");
+ }
+
+ @Test
+ @DisplayName("a member added by code clears the invite they came with")
+ void codeMemberClearsTheInvite() {
+ guild.inviteMember(playerId);
+
+ guild.addMemberByCode(new GuildMember(playerId, MEMBER));
+
+ assertFalse(guild.checkIfInvited(invitedPlayer()), "the invite should have been cleared");
+ }
+
+ @Test
+ @DisplayName("a member who is already in the guild is not added twice")
+ void aMemberIsOnlyInTheGuildOnce() {
+ guild.addMemberByCode(new GuildMember(playerId, MEMBER));
+ guild.addMemberByCode(new GuildMember(playerId, MEMBER));
+
+ assertEquals(1, guild.getMembers().stream().filter(m -> m.getUuid().equals(playerId)).count());
+ }
+
+ @Test
+ @DisplayName("a member added by code keeps the role the caller chose")
+ void codeMemberKeepsItsRole() {
+ guild.addMemberByCode(new GuildMember(playerId, MEMBER));
+
+ assertEquals(MEMBER, guild.getMember(playerId).getRole());
+ }
+
+ @Test
+ @DisplayName("an invited member is registered the same way a code member is")
+ void invitedMemberIsRegisteredTheSameWay() {
+ final GuildHandler handler = Mockito.mock(GuildHandler.class);
+ Mockito.when(handler.getLowestGuildRole()).thenReturn(MEMBER);
+
+ final OfflinePlayer player = Mockito.mock(OfflinePlayer.class);
+ Mockito.when(player.getUniqueId()).thenReturn(playerId);
+ guild.inviteMember(playerId);
+
+ guild.addMember(player, handler);
+
+ final GuildMember member = guild.getMember(playerId);
+ assertNotNull(member);
+ assertTrue(member.getJoinDate() > 0L, "the member should have a join date");
+ assertFalse(guild.checkIfInvited(invitedPlayer()), "the invite should have been cleared");
+ }
+
+ @Test
+ @DisplayName("a player who is already in the guild is not added a second time")
+ void anInvitedPlayerIsNotAddedTwice() {
+ final GuildHandler handler = Mockito.mock(GuildHandler.class);
+ Mockito.when(handler.getLowestGuildRole()).thenReturn(MEMBER);
+
+ final OfflinePlayer player = Mockito.mock(OfflinePlayer.class);
+ Mockito.when(player.getUniqueId()).thenReturn(playerId);
+
+ guild.addMember(player, handler);
+ guild.addMember(player, handler);
+
+ assertEquals(1, guild.getMembers().stream().filter(m -> m.getUuid().equals(playerId)).count());
+ }
+
+ @Test
+ @DisplayName("a member who is not in the guild is not found")
+ void anUnknownMemberIsNotFound() {
+ assertNull(guild.getMember(playerId));
+ }
+
+ /**
+ * The invited player, as the guild sees them.
+ */
+ private Player invitedPlayer() {
+ final Player player = Mockito.mock(Player.class);
+ Mockito.when(player.getUniqueId()).thenReturn(playerId);
+ return player;
+ }
+}