diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index 10d87eacd..2e47b7d43 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -3,7 +3,7 @@ Which Guilds feature is tested where, and what is deliberately left out. See `e2e-testing.md` for how to run any of it. -141 tests across 12 spec files, plus 3 preflight tests that run before every suite and fail the run +162 tests across 14 spec files, plus 3 preflight tests that run before every suite and fail the run if Guilds or any of its hard dependencies did not load. ## Preflight — `plugins/guilds-preflight.spec.ts` @@ -17,12 +17,12 @@ meaningful. | The economy and permission providers Vault needs are registered | Vault's `Economy` and `Permission` services resolve | | Guilds commands are reachable and its language is English | `/guild` answers, and does so in English | -## A. Guild lifecycle — `tests/lifecycle.spec.ts` (19) +## A. Guild lifecycle — `tests/lifecycle.spec.ts` (21) | Requirement | Test | | --- | --- | | Successful guild creation | creates a guild, charges for it, and shows it in the guild list | -| Creation confirmation and cancellation | cancelling the prompt leaves no guild and no charge | +| Creation confirmation and cancellation | cancelling the prompt leaves no guild and no charge / refuses the confirmation when the player joined a guild in the meantime / refuses the confirmation when another guild took the name in the meantime | | Invalid guild names and prefixes | rejects a guild name that breaks the configured requirements / rejects a name too long to serve as its own prefix / rejects an explicit prefix that breaks the prefix requirements | | Duplicate guild names | rejects a guild name another guild already holds | | Insufficient funds | rejects creation when the player cannot afford it | @@ -32,7 +32,7 @@ meaningful. | Status and information commands | toggles the guild between public and private / changes the guild prefix / sets, shows and clears the guild MOTD / reports guild membership through the guild list and rejects guild-scoped commands elsewhere | | Name blacklist | rejects a guild name that is a substring of a blacklisted word / accepts a guild name that contains a blacklisted word | -## B. Membership management — `tests/membership.spec.ts` (19) +## B. Membership management — `tests/membership.spec.ts` (21) | Requirement | Test | | --- | --- | @@ -49,9 +49,10 @@ meaningful. | Promoting and demoting | promotion and demotion change what a member may do / promotion is refused when there is no higher role to move into / a member without the promote role is stopped before the command runs | | Transferring ownership | ownership can be transferred, and the old master loses the master role | | Guild capacity | a full guild refuses another member | +| Finding a guild by name | a guild is found when its name is typed in a different case | | Permissions after leaving or being removed | a player who leaves loses the permissions their guild granted them / a player who is kicked loses the permissions their guild granted them | -## C. Roles and permissions — `tests/roles.spec.ts` (8), `tests/admin.spec.ts` (7) +## C. Roles and permissions — `tests/roles.spec.ts` (8), `tests/admin.spec.ts` (8) | Requirement | Test | | --- | --- | @@ -64,6 +65,7 @@ meaningful. | Tier permissions after upgrade | upgrading charges the tier price and moves the guild onto the new tier / **an upgrade leaves the master with the permissions the new tier grants** | | A player in no guild | a player in no guild is told so rather than refused by role | | Admin commands that name a guild and a second argument | an admin command that names a guild and a second argument reaches its handler / an admin command that names a guild which does not exist says so / an admin command that names a guild and a second argument is refused for an unknown player | +| Admin commands that name a guild | an admin command finds its guild when the name is typed in a different case | | Admin commands that act on a player | an admin command can put a player into a guild / an admin command can hand a player upgrade tickets / an admin command can hand a guild to one of its members | | Admin commands that act on an arena | an admin command can teleport to an arena spawn point | | The player-facing bank commands | the player-facing bank commands work | @@ -128,15 +130,19 @@ meaningful. | Alliance chat between guilds | `chat.spec.ts`, ally chat reaches an allied guild and not the third party | | Cleanup on deletion | a deleted guild drops out of its allies | -## H. Invite codes — `tests/codes.spec.ts` (12, of which 5 are one `describe.serial` chain) +## H. Invite codes — `tests/codes.spec.ts` (18, of which 5 are one `describe.serial` chain) | Requirement | Test | | --- | --- | | Creating invite codes | a code can be created, listed and deleted | | Viewing and listing codes | a code can be created, listed and deleted / *and it is listed for the guild master* | | Redeeming a code | a code can be redeemed and puts the player in the guild | +| Redeeming a code more than once | a code with two uses can be redeemed twice | +| Redeeming a code grants what the guild grants | a player who joins by code gets the permissions their guild grants them | +| The join cooldown applies to a code | a code cannot be redeemed while the join cooldown is running | | Deleting codes | a code can be created, listed and deleted | | Invalid codes | an invalid code is refused | +| Use counts that could never be redeemed | a code with no uses is refused and nothing is stored / a code with a negative number of uses is refused and nothing is stored / a refused code does not use up the guild allowance of codes | | Reusing codes where restricted | a code runs out of uses and then says so | | Permission restrictions | code information is restricted by role / *a member can be added and cannot manage codes* | | Full lifecycle | *one code through its whole life* — a `describe.serial` chain: create → listed → redeem → deleted, asserting membership at each step rather than the command's reply | @@ -188,6 +194,10 @@ the server answering in another language for the rest of the run. ACF's `usePerI false)` is what makes that safe, and one player switching language leaves everyone else in English asserts it rather than assuming it. +## L. Startup and the guild data — `tests/startup-loading.spec.ts` (4) + +## Runtime smoke coverage — `tests/smoke.spec.ts` (7) + ## Fixture self-check — `tests/bank.spec.ts` `STARTING_BALANCE is what the fixtures actually fund` is not a Guilds test. It checks that the diff --git a/src/main/java/me/glaremasters/guilds/guild/Guild.java b/src/main/java/me/glaremasters/guilds/guild/Guild.java index eeb950ab9..747e045dc 100644 --- a/src/main/java/me/glaremasters/guilds/guild/Guild.java +++ b/src/main/java/me/glaremasters/guilds/guild/Guild.java @@ -391,8 +391,13 @@ public List getActiveCodes() { * @param code the code being added * @param uses the amount of uses it has * @param creator the creator of the code + * @throws IllegalArgumentException if the code would have no uses left to begin with */ public void addCode(String code, int uses, Player creator) { + if (uses < GuildCode.MIN_USES) { + throw new IllegalArgumentException( + "An invite code needs at least " + GuildCode.MIN_USES + " use, but got " + uses); + } codes.add(new GuildCode(code, uses, creator.getUniqueId(), new ArrayList<>())); } 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..349881620 100644 --- a/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt +++ b/src/main/kotlin/me/glaremasters/guilds/commands/codes/CommandCode.kt @@ -17,6 +17,7 @@ import me.glaremasters.guilds.Guilds import me.glaremasters.guilds.configuration.sections.CodeSettings import me.glaremasters.guilds.exceptions.ExpectationNotMet import me.glaremasters.guilds.guild.Guild +import me.glaremasters.guilds.guild.GuildCode import me.glaremasters.guilds.guild.GuildHandler import me.glaremasters.guilds.messages.Messages import me.glaremasters.guilds.utils.Constants @@ -38,6 +39,10 @@ internal class CommandCode : BaseCommand() { @Syntax("%uses") @CommandPermission(Constants.CODE_PERM + "create") fun create(player: Player, @Conditions("perm:perm=CREATE_CODE") guild: Guild, @Default("1") uses: Int) { + if (uses < GuildCode.MIN_USES) { + throw ExpectationNotMet(Messages.CODES__INVALID_USES) + } + if (guild.getActiveCheck(settingsManager.getProperty(CodeSettings.ACTIVE_CODE_AMOUNT))) { throw ExpectationNotMet(Messages.CODES__MAX) } diff --git a/src/main/kotlin/me/glaremasters/guilds/guild/GuildCode.kt b/src/main/kotlin/me/glaremasters/guilds/guild/GuildCode.kt index c74d5bc9f..542d018dd 100644 --- a/src/main/kotlin/me/glaremasters/guilds/guild/GuildCode.kt +++ b/src/main/kotlin/me/glaremasters/guilds/guild/GuildCode.kt @@ -18,6 +18,16 @@ class GuildCode( val redeemers: MutableList ) { + companion object { + /** + * The smallest number of uses a code can be created with. + * + * A code that has been redeemed down to zero is a normal, spent code, so the rule belongs at + * creation: a count that could never be redeemed is refused before the code is stored. + */ + const val MIN_USES = 1 + } + /** * Adds a player to the list of redeemers and reduces the number of uses by 1. * diff --git a/src/main/kotlin/me/glaremasters/guilds/messages/Messages.kt b/src/main/kotlin/me/glaremasters/guilds/messages/Messages.kt index 982a0da8b..1b3e56276 100644 --- a/src/main/kotlin/me/glaremasters/guilds/messages/Messages.kt +++ b/src/main/kotlin/me/glaremasters/guilds/messages/Messages.kt @@ -213,6 +213,7 @@ enum class Messages : MessageKeyProvider { CODES__OUT, CODES__INFO, CODES__MAX, + CODES__INVALID_USES, CODES__GUILD_MESSAGE, VAULTS__MAXED, diff --git a/src/main/resources/languages/en-US.yml b/src/main/resources/languages/en-US.yml index 79a069783..0e199cabe 100644 --- a/src/main/resources/languages/en-US.yml +++ b/src/main/resources/languages/en-US.yml @@ -298,6 +298,7 @@ codes: out: "&cThat invite code is all out of uses! Please try another one!" info: "&aInvite Code Information:\n&aID: &6{code}\n&aUses: &6{amount}\n&aCreator: &6{creator}\n&aRedeemers: &6{redeemers}" max: "&cYou already have too many active invite codes in your guild!" + invalid-uses: "&cThe number of uses must be at least 1!" vaults: maxed: "&cYou can't open this vault because you already have the max amount of vaults for your Guild Tier!" blacklisted: "&cYou can't put that item in your Guild Vault because it is blacklisted!" diff --git a/src/test/e2e/plugins/support/expected.ts b/src/test/e2e/plugins/support/expected.ts index 18d369029..fad0d69a9 100644 --- a/src/test/e2e/plugins/support/expected.ts +++ b/src/test/e2e/plugins/support/expected.ts @@ -196,6 +196,8 @@ export const MSG = { codeJoined: (guild: string) => `You've successfully joined ${guild} using an invite code!`, codeOut: 'That invite code is all out of uses!', codeMax: 'You already have too many active invite codes in your guild!', + /** codes.invalid-uses — what a code creation with no uses to offer is answered with. */ + codeInvalidUses: 'The number of uses must be at least 1!', // motd motdNotSet: 'Your guild currently does not have a MOTD set', diff --git a/src/test/e2e/tests/codes.spec.ts b/src/test/e2e/tests/codes.spec.ts index 2e51faf30..2fe5b1b73 100644 --- a/src/test/e2e/tests/codes.spec.ts +++ b/src/test/e2e/tests/codes.spec.ts @@ -114,6 +114,69 @@ test('a guild cannot hold more active codes than the configured limit', async ({ await commandWithout(player, guilds, '/guild code create', MSG.codeMax, MSG.codeCreated); }); +test('a code with no uses is refused and nothing is stored', async ({ player, guilds }) => { + await guilds.createGuild(player); + + await commandWithout(player, guilds, '/guild code create 0', MSG.codeInvalidUses, MSG.codeCreated); + + // Nothing was stored, so the guild has no codes at all. + await commandWithout(player, guilds, '/guild code list', MSG.codeEmpty, MSG.codeListHeader); +}); + +test('a code with a negative number of uses is refused and nothing is stored', async ({ player, guilds }) => { + await guilds.createGuild(player); + + await commandWithout(player, guilds, '/guild code create -1', MSG.codeInvalidUses, MSG.codeCreated); + + // Nothing was stored, so the guild has no codes at all. + await commandWithout(player, guilds, '/guild code list', MSG.codeEmpty, MSG.codeListHeader); +}); + +test('a refused code does not use up the guild allowance of codes', async ({ player, guilds }) => { + await guilds.createGuild(player); + + await commandWithout(player, guilds, '/guild code create 0', MSG.codeInvalidUses, MSG.codeCreated); + await commandWithout(player, guilds, '/guild code create -1', MSG.codeInvalidUses, MSG.codeCreated); + + // codes.amount is two in the staged config, and neither refusal took a slot. + for (let index = 0; index < 2; index++) { + const since = player.getMessageBufferIndex(); + await guilds.run(player, '/guild code create'); + await expectReceived(player, MSG.codeCreated, since); + } + + await commandWithout(player, guilds, '/guild code create', MSG.codeMax, MSG.codeCreated); +}); + +test('a code with two uses can be redeemed twice', 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 first = await createPlayer({ username: guilds.playerName() }); + const joining = first.getMessageBufferIndex(); + await guilds.run(first, `/guild code redeem ${code}`); + await expectReceived(first, MSG.codeJoined(guild), joining); + + // Tier 1 allows two members, so the first joiner has to make room for the second. + await guilds.run(player, `/guild kick ${first.username}`); + await expect(player).toHaveReceivedMessage(MSG.kickSuccess(first.username)); + + const second = await createPlayer({ username: guilds.playerName() }); + const secondJoining = second.getMessageBufferIndex(); + await guilds.run(second, `/guild code redeem ${code}`); + await expectReceived(second, MSG.codeJoined(guild), secondJoining); + + // Both uses are spent now, so the code is refused. + await guilds.run(player, `/guild kick ${second.username}`); + await expect(player).toHaveReceivedMessage(MSG.kickSuccess(second.username)); + + const third = await createPlayer({ username: guilds.playerName() }); + await commandWithout(third, guilds, `/guild code redeem ${code}`, MSG.codeOut, MSG.codeJoined(guild)); +}); + test('code information is restricted by role', async ({ player, createPlayer, guilds }) => { const guild = await guilds.createGuild(player); const created = player.getMessageBufferIndex(); diff --git a/src/test/java/me/glaremasters/guilds/guild/GuildCodeUsageTest.java b/src/test/java/me/glaremasters/guilds/guild/GuildCodeUsageTest.java new file mode 100644 index 000000000..7ab69ca99 --- /dev/null +++ b/src/test/java/me/glaremasters/guilds/guild/GuildCodeUsageTest.java @@ -0,0 +1,228 @@ +package me.glaremasters.guilds.guild; + +import ch.jalu.configme.SettingsManager; +import co.aikar.commands.CommandIssuer; +import co.aikar.commands.CommandManager; +import co.aikar.commands.CommandOperationContext; +import co.aikar.locales.MessageKey; +import me.glaremasters.guilds.commands.codes.CommandCode; +import me.glaremasters.guilds.configuration.sections.CodeSettings; +import me.glaremasters.guilds.exceptions.ExpectationNotMet; +import org.bukkit.entity.Player; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Stack; +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.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Covers the number of uses an invite code may be created with. + * + *

{@code /guild code create} takes the number of uses as an argument. Nothing checked it, so a + * guild master could ask for zero uses or for a negative number. Both produced a code that the + * plugin could never redeem: {@code Guild#getActiveCodes()} only counts codes with uses above + * zero, so such a code was invisible to the active-code limit and to the code list, and + * {@code GuildHandler#handleInvite} refuses to redeem anything with no uses left. A guild could + * therefore fill its storage with codes that neither counted towards its limit nor ever worked. + * + *

The rule is that a code has to be created with at least one use. A code that has been + * redeemed down to zero is a normal, spent code and stays exactly as it is. + */ +class GuildCodeUsageTest { + + /** The message key the refusal carries, so the player sees the same thing everywhere. */ + private static final String INVALID_USES_KEY = "codes.invalid-uses"; + + /** The active-code limit the tests pretend the server has configured. */ + private static final int ACTIVE_CODE_LIMIT = 2; + + @SuppressWarnings("rawtypes") + private ThreadLocal> commandContext; + + @AfterEach + void clearCommandContext() throws Exception { + if (commandContext != null) { + commandContext.remove(); + } + } + + @Test + @DisplayName("a code with no uses is refused and nothing is stored") + void zeroUsesIsRefused() throws Exception { + final Guild guild = guild(); + final CommandCode command = command(); + + final ExpectationNotMet refusal = assertThrows(ExpectationNotMet.class, + () -> command.create(player(), guild, 0), + "a code with no uses must be refused"); + + assertEquals(INVALID_USES_KEY, messageKeyOf(refusal)); + assertTrue(guild.getCodes().isEmpty(), "the refused code must not be stored"); + assertFalse(guild.getActiveCheck(ACTIVE_CODE_LIMIT), + "the refused code must not count against the active-code limit"); + } + + @Test + @DisplayName("a code with a negative number of uses is refused and nothing is stored") + void negativeUsesIsRefused() throws Exception { + final Guild guild = guild(); + final CommandCode command = command(); + + final ExpectationNotMet refusal = assertThrows(ExpectationNotMet.class, + () -> command.create(player(), guild, -1), + "a code with a negative number of uses must be refused"); + + assertEquals(INVALID_USES_KEY, messageKeyOf(refusal)); + assertTrue(guild.getCodes().isEmpty(), "the refused code must not be stored"); + assertFalse(guild.getActiveCheck(ACTIVE_CODE_LIMIT), + "the refused code must not count against the active-code limit"); + } + + @Test + @DisplayName("a refused code leaves the guild's whole allowance of codes") + void refusedCodeLeavesTheAllowanceAlone() throws Exception { + final Guild guild = guild(); + final CommandCode command = command(); + + assertThrows(ExpectationNotMet.class, () -> command.create(player(), guild, 0)); + assertThrows(ExpectationNotMet.class, () -> command.create(player(), guild, -1)); + + // The guild can still create as many codes as the limit allows. + for (int index = 0; index < ACTIVE_CODE_LIMIT; index++) { + command.create(player(), guild, 1); + } + + assertEquals(ACTIVE_CODE_LIMIT, guild.getCodes().size()); + assertTrue(guild.getActiveCheck(ACTIVE_CODE_LIMIT)); + } + + @Test + @DisplayName("a code with one use is created and can be redeemed once") + void oneUseCodeIsCreated() throws Exception { + final Guild guild = guild(); + final CommandCode command = command(); + + command.create(player(), guild, 1); + + assertEquals(1, guild.getCodes().size()); + assertEquals(1, guild.getCodes().get(0).getUses()); + assertEquals(1, guild.getActiveCodes().size()); + + guild.getCodes().get(0).addRedeemer(player()); + + assertEquals(0, guild.getCodes().get(0).getUses()); + assertTrue(guild.getActiveCodes().isEmpty(), "a spent code is no longer an active code"); + } + + @Test + @DisplayName("a code with two uses can be redeemed twice") + void twoUseCodeIsCreated() throws Exception { + final Guild guild = guild(); + final CommandCode command = command(); + + command.create(player(), guild, 2); + + assertEquals(1, guild.getCodes().size()); + assertEquals(2, guild.getCodes().get(0).getUses()); + + final GuildCode code = guild.getCodes().get(0); + code.addRedeemer(player()); + assertEquals(1, code.getUses()); + code.addRedeemer(player()); + assertEquals(0, code.getUses()); + } + + @Test + @DisplayName("the guild refuses to store a code with no uses of its own accord") + void guildRefusesToStoreAnImpossibleCode() { + final Guild guild = guild(); + + assertThrows(IllegalArgumentException.class, () -> guild.addCode("DEAD1", 0, player()), + "a code with no uses cannot be stored"); + assertThrows(IllegalArgumentException.class, () -> guild.addCode("DEAD2", -1, player()), + "a code with a negative number of uses cannot be stored"); + + assertTrue(guild.getCodes().isEmpty()); + assertTrue(guild.getActiveCodes().isEmpty()); + assertFalse(guild.getActiveCheck(ACTIVE_CODE_LIMIT)); + } + + /** + * Builds a guild with an empty set of codes, the way a guild that has never made one looks. + * + * @return a guild whose code list is empty + */ + private static Guild guild() { + final Guild guild = new Guild(UUID.randomUUID()); + guild.setCodes(new ArrayList<>()); + return guild; + } + + /** + * Builds the command under test, with the settings and the command issuer it needs. + * + *

The issuer is what the command sends its confirmation through. ACF keeps the issuer for + * the command being executed in a thread local, so the test puts one there. + * + * @return a command ready to be called + * @throws Exception if ACF's internals cannot be reached + */ + @SuppressWarnings({"unchecked", "rawtypes"}) + private CommandCode command() throws Exception { + final SettingsManager settings = mock(SettingsManager.class); + when(settings.getProperty(CodeSettings.ACTIVE_CODE_AMOUNT)).thenReturn(ACTIVE_CODE_LIMIT); + when(settings.getProperty(CodeSettings.CODE_LENGTH)).thenReturn(7); + + final CommandCode command = new CommandCode(); + command.setSettingsManager(settings); + + final Field field = CommandManager.class.getDeclaredField("commandOperationContext"); + field.setAccessible(true); + this.commandContext = (ThreadLocal>) field.get(null); + + final CommandOperationContext context = mock(CommandOperationContext.class); + when(context.getCommandIssuer()).thenReturn(mock(CommandIssuer.class)); + + final Stack stack = new Stack<>(); + stack.push(context); + commandContext.set(stack); + + return command; + } + + /** + * A player who created a code. Nothing but the identity is ever asked of them. + * + * @return a player with a fresh identity + */ + private static Player player() { + final Player player = mock(Player.class); + when(player.getUniqueId()).thenReturn(UUID.randomUUID()); + return player; + } + + /** + * The message key an ACF argument failure carries. + * + *

ACF keeps the key in a package private field, so it has to be read reflectively. + * + * @param failure the failure the command raised + * @return the key the player is shown + * @throws Exception if the field cannot be read + */ + private static String messageKeyOf(ExpectationNotMet failure) throws Exception { + final Field field = failure.getClass().getSuperclass().getDeclaredField("key"); + field.setAccessible(true); + return ((MessageKey) field.get(failure)).getKey(); + } +}