Skip to content

Commit

Permalink
Ensure label cannot be null in ButtonImpl
Browse files Browse the repository at this point in the history
  • Loading branch information
MinnDevelopment committed Nov 10, 2024
1 parent 466a66b commit 2c0bfeb
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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();
}

/**
Expand All @@ -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();
}

/**
Expand Down Expand Up @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -81,32 +82,27 @@ 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;
case LINK:
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;
case PREMIUM:
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;
}

Expand Down
46 changes: 24 additions & 22 deletions src/test/java/net/dv8tion/jda/test/interactions/ButtonTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -48,22 +49,23 @@ void testButtonValid(ButtonStyle style, String id, String label, String url, Sku
{
ButtonImpl button = new ButtonImpl(id, label, style, url, sku, false, emoji);
assertDoesNotThrow(button::checkValid);
assertDoesNotThrow(button::toData);
}

static Stream<Arguments> testButtonValid()
{
// 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)
);
}

Expand All @@ -80,20 +82,20 @@ static Stream<Arguments> 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)
);
}

Expand All @@ -108,14 +110,14 @@ void testPrimaryWithSku()
void testPrimaryWithUrl()
{
assertThatIllegalArgumentException()
.isThrownBy(() -> Button.primary("id", EXAMPLE_LABEL).withUrl(EXAMPLE_URL));
.isThrownBy(() -> Button.primary("id", EXAMPLE_LABEL).withUrl(EXAMPLE_URL));
}

@Test
void linkWithId()
{
assertThatIllegalArgumentException()
.isThrownBy(() -> Button.link(EXAMPLE_URL, EXAMPLE_LABEL).withId(EXAMPLE_ID));
.isThrownBy(() -> Button.link(EXAMPLE_URL, EXAMPLE_LABEL).withId(EXAMPLE_ID));
}

@Test
Expand All @@ -129,7 +131,7 @@ void linkWithSku()
void testPremiumWithId()
{
assertThatIllegalArgumentException()
.isThrownBy(() -> Button.premium(EXAMPLE_SKU).withLabel(EXAMPLE_ID));
.isThrownBy(() -> Button.premium(EXAMPLE_SKU).withLabel(EXAMPLE_ID));
}

@Test
Expand Down

0 comments on commit 2c0bfeb

Please sign in to comment.