From 51f907c8305e30f4cfb81ec7058bf782919e209e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Spie=C3=9F?= Date: Sun, 10 Nov 2024 12:16:24 +0100 Subject: [PATCH] Ensure label cannot be null in ButtonImpl (#2771) --- .../components/buttons/Button.java | 34 ++-- .../interactions/component/ButtonImpl.java | 16 +- .../jda/test/interactions/ButtonTests.java | 192 +++++++++++++----- 3 files changed, 163 insertions(+), 79 deletions(-) diff --git a/src/main/java/net/dv8tion/jda/api/interactions/components/buttons/Button.java b/src/main/java/net/dv8tion/jda/api/interactions/components/buttons/Button.java index deddd28f0c..dcf7628394 100644 --- a/src/main/java/net/dv8tion/jda/api/interactions/components/buttons/Button.java +++ b/src/main/java/net/dv8tion/jda/api/interactions/components/buttons/Button.java @@ -171,7 +171,7 @@ default Button withDisabled(boolean disabled) @CheckReturnValue default Button withEmoji(@Nullable Emoji emoji) { - return new ButtonImpl(getId(), getLabel(), getStyle(), getUrl(), null, isDisabled(), emoji).checkValid(); + return new ButtonImpl(getId(), getLabel(), getStyle(), getUrl(), getSku(), isDisabled(), emoji).checkValid(); } /** @@ -217,7 +217,7 @@ default Button withLabel(@Nonnull String label) @CheckReturnValue default Button withId(@Nonnull String id) { - return new ButtonImpl(id, getLabel(), getStyle(), null, null, isDisabled(), getEmoji()).checkValid(); + return new ButtonImpl(id, getLabel(), getStyle(), getUrl(), getSku(), isDisabled(), getEmoji()).checkValid(); } /** @@ -240,7 +240,7 @@ default Button withId(@Nonnull String id) @CheckReturnValue default Button withUrl(@Nonnull String url) { - return new ButtonImpl(null, getLabel(), getStyle(), url, null, isDisabled(), getEmoji()).checkValid(); + return new ButtonImpl(getId(), getLabel(), getStyle(), url, getSku(), isDisabled(), getEmoji()).checkValid(); } /** @@ -258,7 +258,7 @@ default Button withUrl(@Nonnull String url) @CheckReturnValue default Button withSku(@Nonnull SkuSnowflake sku) { - return new ButtonImpl(null, "", getStyle(), null, sku, isDisabled(), null).checkValid(); + return new ButtonImpl(getId(), getLabel(), getStyle(), getUrl(), sku, isDisabled(), getEmoji()).checkValid(); } /** @@ -345,7 +345,7 @@ static Button primary(@Nonnull String id, @Nonnull String label) @Nonnull static Button primary(@Nonnull String id, @Nonnull Emoji emoji) { - return new ButtonImpl(id, "", ButtonStyle.PRIMARY, false, emoji).checkValid(); + return new ButtonImpl(id, null, ButtonStyle.PRIMARY, false, emoji).checkValid(); } /** @@ -399,7 +399,7 @@ static Button secondary(@Nonnull String id, @Nonnull String label) @Nonnull static Button secondary(@Nonnull String id, @Nonnull Emoji emoji) { - return new ButtonImpl(id, "", ButtonStyle.SECONDARY, false, emoji).checkValid(); + return new ButtonImpl(id, null, ButtonStyle.SECONDARY, false, emoji).checkValid(); } /** @@ -453,7 +453,7 @@ static Button success(@Nonnull String id, @Nonnull String label) @Nonnull static Button success(@Nonnull String id, @Nonnull Emoji emoji) { - return new ButtonImpl(id, "", ButtonStyle.SUCCESS, false, emoji).checkValid(); + return new ButtonImpl(id, null, ButtonStyle.SUCCESS, false, emoji).checkValid(); } /** @@ -507,7 +507,7 @@ static Button danger(@Nonnull String id, @Nonnull String label) @Nonnull static Button danger(@Nonnull String id, @Nonnull Emoji emoji) { - return new ButtonImpl(id, "", ButtonStyle.DANGER, false, emoji).checkValid(); + return new ButtonImpl(id, null, ButtonStyle.DANGER, false, emoji).checkValid(); } /** @@ -567,7 +567,7 @@ static Button link(@Nonnull String url, @Nonnull String label) @Nonnull static Button link(@Nonnull String url, @Nonnull Emoji emoji) { - return new ButtonImpl(null, "", ButtonStyle.LINK, url, null, false, emoji).checkValid(); + return new ButtonImpl(null, null, ButtonStyle.LINK, url, null, false, emoji).checkValid(); } /** @@ -589,7 +589,7 @@ static Button link(@Nonnull String url, @Nonnull Emoji emoji) @Nonnull static Button premium(@Nonnull SkuSnowflake sku) { - return new ButtonImpl(null, "", ButtonStyle.PREMIUM, null, sku, false, null).checkValid(); + return new ButtonImpl(null, null, ButtonStyle.PREMIUM, null, sku, false, null).checkValid(); } /** @@ -661,7 +661,7 @@ static Button of(@Nonnull ButtonStyle style, @Nonnull String idOrUrl, @Nonnull E Checks.check(style != ButtonStyle.PREMIUM, "Premium buttons don't support emojis"); if (style == ButtonStyle.LINK) return link(idOrUrl, emoji); - return new ButtonImpl(idOrUrl, "", style, false, emoji).checkValid(); + return new ButtonImpl(idOrUrl, null, style, false, emoji).checkValid(); } /** @@ -697,10 +697,16 @@ static Button of(@Nonnull ButtonStyle style, @Nonnull String idOrUrl, @Nonnull E @Nonnull static Button of(@Nonnull ButtonStyle style, @Nonnull String idOrUrlOrSku, @Nullable String label, @Nullable Emoji emoji) { - if (style == ButtonStyle.LINK) + Checks.notNull(style, "ButtonStyle"); + + switch (style) + { + case LINK: return new ButtonImpl(null, label, style, idOrUrlOrSku, null, false, emoji).checkValid(); - if (style == ButtonStyle.PREMIUM) + case PREMIUM: return new ButtonImpl(null, label, style, null, SkuSnowflake.fromId(idOrUrlOrSku), false, emoji).checkValid(); - return new ButtonImpl(idOrUrlOrSku, label, style, null, null, false, emoji).checkValid(); + default: + return new ButtonImpl(idOrUrlOrSku, label, style, null, null, false, emoji).checkValid(); + } } } diff --git a/src/main/java/net/dv8tion/jda/internal/interactions/component/ButtonImpl.java b/src/main/java/net/dv8tion/jda/internal/interactions/component/ButtonImpl.java index c2d81caa94..ab407c6ea6 100644 --- a/src/main/java/net/dv8tion/jda/internal/interactions/component/ButtonImpl.java +++ b/src/main/java/net/dv8tion/jda/internal/interactions/component/ButtonImpl.java @@ -60,9 +60,9 @@ public ButtonImpl(String id, String label, ButtonStyle style, boolean disabled, public ButtonImpl(String id, String label, ButtonStyle style, String url, SkuSnowflake sku, boolean disabled, Emoji emoji) { this.id = id; - this.label = label; + this.label = label == null ? "" : label; this.style = style; - this.url = url; // max length 512 + this.url = url; this.sku = sku; this.disabled = disabled; this.emoji = (EmojiUnion) emoji; @@ -71,6 +71,7 @@ public ButtonImpl(String id, String label, ButtonStyle style, String url, SkuSno public ButtonImpl checkValid() { Checks.notNull(style, "Style"); + Checks.notLonger(label, LABEL_MAX_LENGTH, "Label"); Checks.check(style != ButtonStyle.UNKNOWN, "Cannot make button with unknown style!"); switch (style) @@ -81,7 +82,7 @@ public ButtonImpl checkValid() case DANGER: Checks.check(url == null, "Cannot set an URL on action buttons"); Checks.check(sku == null, "Cannot set an SKU on action buttons"); - Checks.check(emoji != null || (label != null && !label.isEmpty()), "Action buttons must have either an emoji or label"); + Checks.check(emoji != null || !label.isEmpty(), "Action buttons must have either an emoji or label"); Checks.notEmpty(id, "Id"); Checks.notLonger(id, ID_MAX_LENGTH, "Id"); break; @@ -89,7 +90,7 @@ public ButtonImpl checkValid() Checks.check(id == null, "Cannot set an ID on link buttons"); Checks.check(url != null, "You must set an URL on link buttons"); Checks.check(sku == null, "Cannot set an SKU on link buttons"); - Checks.check(emoji != null || (label != null && !label.isEmpty()), "Link buttons must have either an emoji or label"); + Checks.check(emoji != null || !label.isEmpty(), "Link buttons must have either an emoji or label"); Checks.notEmpty(url, "URL"); Checks.notLonger(url, URL_MAX_LENGTH, "URL"); break; @@ -97,16 +98,11 @@ public ButtonImpl checkValid() Checks.check(id == null, "Cannot set an ID on premium buttons"); Checks.check(url == null, "Cannot set an URL on premium buttons"); Checks.check(emoji == null, "Cannot set an emoji on premium buttons"); - Checks.check(label == null || label.isEmpty(), "Cannot set a label on premium buttons"); + Checks.check(label.isEmpty(), "Cannot set a label on premium buttons"); Checks.notNull(sku, "SKU"); break; } - if (label != null) - { - Checks.notLonger(label, LABEL_MAX_LENGTH, "Label"); - } - return this; } diff --git a/src/test/java/net/dv8tion/jda/test/interactions/ButtonTests.java b/src/test/java/net/dv8tion/jda/test/interactions/ButtonTests.java index b83667ce39..7b25271072 100644 --- a/src/test/java/net/dv8tion/jda/test/interactions/ButtonTests.java +++ b/src/test/java/net/dv8tion/jda/test/interactions/ButtonTests.java @@ -22,9 +22,9 @@ import net.dv8tion.jda.api.interactions.components.buttons.Button; import net.dv8tion.jda.api.interactions.components.buttons.ButtonStyle; import net.dv8tion.jda.internal.interactions.component.ButtonImpl; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.MethodSource; import java.util.stream.Stream; @@ -32,6 +32,7 @@ import static net.dv8tion.jda.api.interactions.components.buttons.ButtonStyle.*; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.params.provider.Arguments.arguments; @SuppressWarnings("ResultOfMethodCallIgnored") public class ButtonTests @@ -42,28 +43,29 @@ public class ButtonTests private static final String EXAMPLE_LABEL = "Label"; private static final SkuSnowflake EXAMPLE_SKU = SkuSnowflake.fromId(1234); - @MethodSource("testButtonValid") + @MethodSource("validButtons") @ParameterizedTest void testButtonValid(ButtonStyle style, String id, String label, String url, SkuSnowflake sku, Emoji emoji) { ButtonImpl button = new ButtonImpl(id, label, style, url, sku, false, emoji); assertDoesNotThrow(button::checkValid); + assertDoesNotThrow(button::toData); } - static Stream testButtonValid() + static Stream validButtons() { // The following button configurations are valid: return Stream.of( // Normal button; id, either label, emoji, label+emoji - Arguments.of(PRIMARY, "id", EXAMPLE_LABEL, null, null, null), - Arguments.of(PRIMARY, "id", EXAMPLE_LABEL, null, null, EXAMPLE_EMOJI), - Arguments.of(PRIMARY, "id", "", null, null, EXAMPLE_EMOJI), + arguments(PRIMARY, "id", EXAMPLE_LABEL, null, null, null), + arguments(PRIMARY, "id", EXAMPLE_LABEL, null, null, EXAMPLE_EMOJI), + arguments(PRIMARY, "id", null, null, null, EXAMPLE_EMOJI), // Link button; url, either label, emoji, label+emoji - Arguments.of(LINK, null, EXAMPLE_LABEL, EXAMPLE_URL, null, null), - Arguments.of(LINK, null, EXAMPLE_LABEL, EXAMPLE_URL, null, EXAMPLE_EMOJI), - Arguments.of(LINK, null, "", EXAMPLE_URL, null, EXAMPLE_EMOJI), + arguments(LINK, null, EXAMPLE_LABEL, EXAMPLE_URL, null, null), + arguments(LINK, null, EXAMPLE_LABEL, EXAMPLE_URL, null, EXAMPLE_EMOJI), + arguments(LINK, null, null, EXAMPLE_URL, null, EXAMPLE_EMOJI), // Premium button doesn't have anything - Arguments.of(PREMIUM, null, "", null, EXAMPLE_SKU, null) + arguments(PREMIUM, null, null, null, EXAMPLE_SKU, null) ); } @@ -80,78 +82,158 @@ static Stream testButtonInvalid() // The following button configuration will fail when: return Stream.of( // Normal button; has no id, has neither label/emoji, has url, has sku - Arguments.of(PRIMARY, null, EXAMPLE_LABEL, null, null, null), - Arguments.of(PRIMARY, "id", "", null, null, null), - Arguments.of(PRIMARY, "id", EXAMPLE_LABEL, EXAMPLE_URL, null, null), - Arguments.of(PRIMARY, "id", EXAMPLE_LABEL, null, EXAMPLE_SKU, null), + arguments(PRIMARY, null, EXAMPLE_LABEL, null, null, null), + arguments(PRIMARY, "id", "", null, null, null), + arguments(PRIMARY, "id", EXAMPLE_LABEL, EXAMPLE_URL, null, null), + arguments(PRIMARY, "id", EXAMPLE_LABEL, null, EXAMPLE_SKU, null), // Link button; has no url, has id, has sku - Arguments.of(LINK, null, EXAMPLE_LABEL, null, null, null), - Arguments.of(LINK, "id", EXAMPLE_LABEL, EXAMPLE_URL, null, null), - Arguments.of(LINK, null, EXAMPLE_LABEL, EXAMPLE_URL, EXAMPLE_SKU, null), + arguments(LINK, null, EXAMPLE_LABEL, null, null, null), + arguments(LINK, "id", EXAMPLE_LABEL, EXAMPLE_URL, null, null), + arguments(LINK, null, EXAMPLE_LABEL, EXAMPLE_URL, EXAMPLE_SKU, null), // Premium button; has no sku, has id, has url, has label, has emoji - Arguments.of(PREMIUM, null, "", null, null, null), - Arguments.of(PREMIUM, "id", "", null, EXAMPLE_SKU, null), - Arguments.of(PREMIUM, null, "", "url", EXAMPLE_SKU, null), - Arguments.of(PREMIUM, null, EXAMPLE_LABEL, null, EXAMPLE_SKU, null), - Arguments.of(PREMIUM, null, "", null, EXAMPLE_SKU, EXAMPLE_EMOJI) + arguments(PREMIUM, null, "", null, null, null), + arguments(PREMIUM, "id", "", null, EXAMPLE_SKU, null), + arguments(PREMIUM, null, "", "url", EXAMPLE_SKU, null), + arguments(PREMIUM, null, EXAMPLE_LABEL, null, EXAMPLE_SKU, null), + arguments(PREMIUM, null, "", null, EXAMPLE_SKU, EXAMPLE_EMOJI) ); } - @Test - void testPrimaryWithSku() + + @MethodSource + @ParameterizedTest + void testWithId(Button button, boolean shouldThrow) { - assertThatIllegalArgumentException() - .isThrownBy(() -> Button.primary("id", EXAMPLE_LABEL).withSku(EXAMPLE_SKU)); + if (shouldThrow) + assertThatIllegalArgumentException().isThrownBy(() -> button.withId("valid-id")); + else + assertDoesNotThrow(() -> button.withId("valid-id")); } - @Test - void testPrimaryWithUrl() + static Stream testWithId() { - assertThatIllegalArgumentException() - .isThrownBy(() -> Button.primary("id", EXAMPLE_LABEL).withUrl(EXAMPLE_URL)); + return Stream.of( + arguments(Button.primary(EXAMPLE_ID, "Primary"), false), + arguments(Button.link(EXAMPLE_URL, "Link"), true), + arguments(Button.premium(EXAMPLE_SKU), true) + ); } - @Test - void linkWithId() + @MethodSource + @ParameterizedTest + void testWithUrl(Button button, boolean shouldThrow) { - assertThatIllegalArgumentException() - .isThrownBy(() -> Button.link(EXAMPLE_URL, EXAMPLE_LABEL).withId(EXAMPLE_ID)); + if (shouldThrow) + assertThatIllegalArgumentException().isThrownBy(() -> button.withUrl(EXAMPLE_URL)); + else + assertDoesNotThrow(() -> button.withUrl(EXAMPLE_URL)); } - - @Test - void linkWithSku() + + static Stream testWithUrl() { - assertThatIllegalArgumentException() - .isThrownBy(() -> Button.link(EXAMPLE_URL, EXAMPLE_LABEL).withSku(EXAMPLE_SKU)); + return Stream.of( + arguments(Button.primary(EXAMPLE_ID, "Primary"), true), + arguments(Button.link(EXAMPLE_URL, "Link"), false), + arguments(Button.premium(EXAMPLE_SKU), true) + ); } - @Test - void testPremiumWithId() + @MethodSource + @ParameterizedTest + void testWithSku(Button button, boolean shouldThrow) { - assertThatIllegalArgumentException() - .isThrownBy(() -> Button.premium(EXAMPLE_SKU).withLabel(EXAMPLE_ID)); + if (shouldThrow) + assertThatIllegalArgumentException().isThrownBy(() -> button.withSku(EXAMPLE_SKU)); + else + assertDoesNotThrow(() -> button.withSku(EXAMPLE_SKU)); } - @Test - void testPremiumWithLabel() + static Stream testWithSku() { - assertThatIllegalArgumentException() - .isThrownBy(() -> Button.premium(EXAMPLE_SKU).withLabel(EXAMPLE_LABEL)); + return Stream.of( + arguments(Button.primary(EXAMPLE_ID, "Primary"), true), + arguments(Button.link(EXAMPLE_URL, "Link"), true), + arguments(Button.premium(EXAMPLE_SKU), false) + ); } + @MethodSource + @ParameterizedTest + void testWithLabel(Button button, boolean shouldThrow) + { + if (shouldThrow) + assertThatIllegalArgumentException().isThrownBy(() -> button.withLabel(EXAMPLE_LABEL)); + else + assertDoesNotThrow(() -> button.withLabel(EXAMPLE_LABEL)); + } - @Test - void testPremiumWithEmoji() + static Stream testWithLabel() { - assertThatIllegalArgumentException() - .isThrownBy(() -> Button.premium(EXAMPLE_SKU).withEmoji(EXAMPLE_EMOJI)); + return Stream.of( + arguments(Button.primary(EXAMPLE_ID, "Primary"), false), + arguments(Button.link(EXAMPLE_URL, "Link"), false), + arguments(Button.premium(EXAMPLE_SKU), true) + ); } + @MethodSource + @ParameterizedTest + void testWithEmoji(Button button, boolean shouldThrow) + { + if (shouldThrow) + assertThatIllegalArgumentException().isThrownBy(() -> button.withEmoji(EXAMPLE_EMOJI)); + else + assertDoesNotThrow(() -> button.withEmoji(EXAMPLE_EMOJI)); + } - @Test - void testPremiumWithUrl() + static Stream testWithEmoji() + { + return Stream.of( + arguments(Button.primary(EXAMPLE_ID, "Primary"), false), + arguments(Button.link(EXAMPLE_URL, "Link"), false), + arguments(Button.premium(EXAMPLE_SKU), true) + ); + } + + @EnumSource + @ParameterizedTest + void testWithStyleLinkToOther(ButtonStyle style) + { + Button button = Button.link(EXAMPLE_URL, "Label"); + if (style == LINK) + assertDoesNotThrow(() -> button.withStyle(style)); + else + assertThatIllegalArgumentException().isThrownBy(() -> button.withStyle(style)); + } + + @EnumSource + @ParameterizedTest + void testWithStylePremiumToOther(ButtonStyle style) + { + Button button = Button.premium(EXAMPLE_SKU); + if (style == PREMIUM) + assertDoesNotThrow(() -> button.withStyle(style)); + else + assertThatIllegalArgumentException().isThrownBy(() -> button.withStyle(style)); + } + + @EnumSource + @ParameterizedTest + void testWithStyleColorToOther(ButtonStyle style) + { + Button button = Button.primary(EXAMPLE_ID, EXAMPLE_LABEL); + if (style == PREMIUM || style == LINK || style == UNKNOWN) + assertThatIllegalArgumentException().isThrownBy(() -> button.withStyle(style)); + else + assertDoesNotThrow(() -> button.withStyle(style)); + } + + @MethodSource("validButtons") + @ParameterizedTest + void testWithDisabled(ButtonStyle style, String id, String label, String url, SkuSnowflake sku, Emoji emoji) { - assertThatIllegalArgumentException() - .isThrownBy(() -> Button.premium(EXAMPLE_SKU).withUrl(EXAMPLE_URL)); + Button button = new ButtonImpl(id, label, style, url, sku, false, emoji); + assertDoesNotThrow(() -> button.withDisabled(true)); + assertDoesNotThrow(() -> button.withDisabled(false)); } }