Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding a check in reaction methods before calling getTextChannel() #1216

Merged
merged 7 commits into from
Apr 4, 2020
Merged

Adding a check in reaction methods before calling getTextChannel() #1216

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7e9661f
Added a check everytime getTextChannel() would be called
DManstrator Mar 4, 2020
b6403bb
Throwing the correct Exception now, improved docs of the Message clas…
DManstrator Mar 4, 2020
e4af795
Fixing possible NPEs and adding null-users to the throws section
DManstrator Mar 4, 2020
4e4bb0a
Checking for isFromGuild instead of ChannelType#TEXT
DManstrator Mar 6, 2020
5d2d89f
fixed a typo
DManstrator Mar 6, 2020
076d2e4
Reverted check back to isFromType
DManstrator Mar 6, 2020
52a478b
Moved the comment above the code
DManstrator Apr 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions src/main/java/net/dv8tion/jda/api/entities/Message.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,8 @@ default boolean isFromGuild()
* Removes all reactions from this Message.
* <br>This is useful for moderator commands that wish to remove all reactions at once from a specific message.
*
* <p>Please note that you <b>can't</b> clear reactions if this message was sent in a {@link net.dv8tion.jda.api.entities.PrivateChannel PrivateChannel}!
*
* <p><b>Neither success nor failure of this request will affect this Message's {@link #getReactions()} return as Message is immutable.</b>
*
* <p>The following {@link net.dv8tion.jda.api.requests.ErrorResponse ErrorResponses} are possible:
Expand All @@ -1189,7 +1191,7 @@ default boolean isFromGuild()
* in the channel.
* @throws java.lang.IllegalStateException
* If this message was <b>not</b> sent in a
* {@link net.dv8tion.jda.api.entities.TextChannel TextChannel}.
* {@link net.dv8tion.jda.api.entities.Guild Guild}.
*
* @return {@link net.dv8tion.jda.api.requests.RestAction RestAction} - Type: {@link java.lang.Void}
*/
Expand All @@ -1200,6 +1202,8 @@ default boolean isFromGuild()
/**
* Removes all reactions for the specified emoji.
*
* <p>Please note that you <b>can't</b> clear reactions if this message was sent in a {@link net.dv8tion.jda.api.entities.PrivateChannel PrivateChannel}!
*
* <h2>Example</h2>
* <pre><code>
* // custom
Expand Down Expand Up @@ -1232,6 +1236,9 @@ default boolean isFromGuild()
* If the currently logged in account does not have {@link Permission#MESSAGE_MANAGE} in the channel
* @throws IllegalArgumentException
* If provided with null
* @throws java.lang.IllegalStateException
* If this message was <b>not</b> sent in a
* {@link net.dv8tion.jda.api.entities.Guild Guild}.
*
* @return {@link RestAction}
*
Expand All @@ -1244,6 +1251,8 @@ default boolean isFromGuild()
/**
* Removes all reactions for the specified emote.
*
* <p>Please note that you <b>can't</b> clear reactions if this message was sent in a {@link net.dv8tion.jda.api.entities.PrivateChannel PrivateChannel}!
*
* <p>The following {@link net.dv8tion.jda.api.requests.ErrorResponse ErrorResponses} are possible:
* <ul>
* <li>{@link net.dv8tion.jda.api.requests.ErrorResponse#MISSING_ACCESS MISSING_ACCESS}
Expand All @@ -1266,6 +1275,9 @@ default boolean isFromGuild()
* If the currently logged in account does not have {@link Permission#MESSAGE_MANAGE} in the channel
* @throws IllegalArgumentException
* If provided with null
* @throws java.lang.IllegalStateException
* If this message was <b>not</b> sent in a
* {@link net.dv8tion.jda.api.entities.Guild Guild}.
*
* @return {@link RestAction}
*
Expand Down Expand Up @@ -1324,6 +1336,8 @@ default boolean isFromGuild()
/**
* Removes a {@link net.dv8tion.jda.api.entities.User User's} reaction from this Message using an {@link net.dv8tion.jda.api.entities.Emote Emote}.
*
* <p>Please note that you <b>can't</b> remove reactions of other users if this message was sent in a {@link net.dv8tion.jda.api.entities.PrivateChannel PrivateChannel}!
*
* <p>This message instance will not be updated by this operation.
*
* <p>Reactions are the small emoji/emotes below a message that have a counter beside them
Expand Down Expand Up @@ -1364,7 +1378,12 @@ default boolean isFromGuild()
* <li>If the provided {@link net.dv8tion.jda.api.entities.Emote Emote} is fake {@link net.dv8tion.jda.api.entities.Emote#isFake() Emote.isFake()}.</li>
* <li>If the provided {@link net.dv8tion.jda.api.entities.Emote Emote} cannot be used in the current channel.
* See {@link Emote#canInteract(User, MessageChannel)} or {@link Emote#canInteract(Member)} for more information.</li>
* <li>If the provided user is null</li>
* </ul>
* @throws java.lang.IllegalStateException
* If this message was <b>not</b> sent in a
* {@link net.dv8tion.jda.api.entities.Guild Guild}
* <b>and</b> the given user is <b>not</b> the {@link net.dv8tion.jda.api.entities.SelfUser SelfUser}.
*
* @return {@link net.dv8tion.jda.api.requests.RestAction RestAction} - Type: {@link java.lang.Void}
*
Expand Down Expand Up @@ -1431,6 +1450,8 @@ default boolean isFromGuild()
* <br>A reference of unicode emojis can be found here:
* <a href="http://unicode.org/emoji/charts/full-emoji-list.html" target="_blank">Emoji Table</a>.
*
* <p>Please note that you <b>can't</b> remove reactions of other users if this message was sent in a {@link net.dv8tion.jda.api.entities.PrivateChannel PrivateChannel}!
*
* <p>This message instance will not be updated by this operation.
*
* <p>Reactions are the small emoji/emotes below a message that have a counter beside them
Expand Down Expand Up @@ -1468,7 +1489,11 @@ default boolean isFromGuild()
* <li>{@link net.dv8tion.jda.api.Permission#MESSAGE_MANAGE Permission.MESSAGE_MANAGE}</li>
* </ul>
* @throws java.lang.IllegalArgumentException
* If the provided unicode emoji is null or empty.
* If the provided unicode emoji is null or empty or if the provided user is null.
* @throws java.lang.IllegalStateException
* If this message was <b>not</b> sent in a
* {@link net.dv8tion.jda.api.entities.Guild Guild}
* <b>and</b> the given user is <b>not</b> the {@link net.dv8tion.jda.api.entities.SelfUser SelfUser}.
*
* @return {@link net.dv8tion.jda.api.requests.RestAction RestAction} - Type: {@link java.lang.Void}
*
Expand Down
32 changes: 26 additions & 6 deletions src/main/java/net/dv8tion/jda/internal/entities/ReceivedMessage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public RestAction<Void> addReaction(@Nonnull String unicode)
@Override
public RestAction<Void> clearReactions()
{
if (!isFromType(ChannelType.TEXT))
if (!isFromGuild())
throw new IllegalStateException("Cannot clear reactions from a message in a Group or PrivateChannel.");
DManstrator marked this conversation as resolved.
Show resolved Hide resolved
return getTextChannel().clearReactionsById(getId());
}
Expand All @@ -172,13 +172,17 @@ public RestAction<Void> clearReactions()
@Override
public RestAction<Void> clearReactions(@Nonnull String unicode)
{
if (!isFromGuild())
throw new IllegalStateException("Cannot clear reactions from a message in a Group or PrivateChannel.");
return getTextChannel().clearReactionsById(getId(), unicode);
}

@Nonnull
@Override
public RestAction<Void> clearReactions(@Nonnull Emote emote)
{
if (!isFromGuild())
throw new IllegalStateException("Cannot clear reactions from a message in a Group or PrivateChannel.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably remove the group from here since bots can't be in groups

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fully aware of this but since this was the original output and the throws section saying "if was not sent in a TextChannel", I decided to stay with this.

However in my JavaDoc additions, I explicitly talk about private messages since it's easier to understand that than telling it has to come from a TextChannel imo.

TL;DR: I would wait for Minn's response since this is likely to be unwanted.

return getTextChannel().clearReactionsById(getId(), emote);
}

Expand All @@ -193,6 +197,14 @@ public RestAction<Void> removeReaction(@Nonnull Emote emote)
@Override
public RestAction<Void> removeReaction(@Nonnull Emote emote, @Nonnull User user)
{
Checks.notNull(user, "User"); // to prevent NPEs
// check if the passed user is the SelfUser, then the ChannelType doesn't matter
if (user.equals(getJDA().getSelfUser()))
// we can safely remove that
return channel.removeReactionById(getIdLong(), emote);
DManstrator marked this conversation as resolved.
Show resolved Hide resolved

if (!isFromGuild())
throw new IllegalStateException("Cannot remove reactions of others from a message in a Group or PrivateChannel.");
return getTextChannel().removeReactionById(getIdLong(), emote, user);
}

Expand All @@ -207,6 +219,12 @@ public RestAction<Void> removeReaction(@Nonnull String unicode)
@Override
public RestAction<Void> removeReaction(@Nonnull String unicode, @Nonnull User user)
{
Checks.notNull(user, "User");
if (user.equals(getJDA().getSelfUser()))
return channel.removeReactionById(getIdLong(), unicode);

if (!isFromGuild())
throw new IllegalStateException("Cannot remove reactions of others from a message in a Group or PrivateChannel.");
return getTextChannel().removeReactionById(getId(), unicode, user);
}

Expand Down Expand Up @@ -390,7 +408,7 @@ public List<Member> getMentionedMembers(@Nonnull Guild guild)
@Override
public List<Member> getMentionedMembers()
{
if (isFromType(ChannelType.TEXT))
if (isFromGuild())
return getMentionedMembers(getGuild());
else
throw new IllegalStateException("You must specify a Guild for Messages which are not sent from a TextChannel!");
Expand Down Expand Up @@ -524,7 +542,7 @@ else if (mentionable instanceof Member)
final Member member = (Member) mentionable;
return CollectionUtils.containsAny(getMentionedRoles(), member.getRoles());
}
else if (isFromType(ChannelType.TEXT) && mentionable instanceof User)
else if (isFromGuild() && mentionable instanceof User)
{
final Member member = getGuild().getMember((User) mentionable);
return member != null && CollectionUtils.containsAny(getMentionedRoles(), member.getRoles());
Expand Down Expand Up @@ -596,7 +614,7 @@ public String getContentDisplay()
for (User user : getMentionedUsers())
{
String name;
if (isFromType(ChannelType.TEXT) && getGuild().isMember(user))
if (isFromGuild() && getGuild().isMember(user))
name = getGuild().getMember(user).getEffectiveName();
else
name = user.getName();
Expand Down Expand Up @@ -682,15 +700,17 @@ public PrivateChannel getPrivateChannel()
@Override
public TextChannel getTextChannel()
{
if (!isFromType(ChannelType.TEXT))
// this is not really safe but at the moment this is the only GuildChannel with messages
// and a MessageChannel can just be Text or Private so this is assumable as of now
if (!isFromGuild())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should be reverted.

throw new IllegalStateException("This message was not sent in a text channel");
return (TextChannel) channel;
}

@Override
public Category getCategory()
{
return isFromType(ChannelType.TEXT) ? getTextChannel().getParent() : null;
return isFromGuild() ? getTextChannel().getParent() : null;
}

@Nonnull
Expand Down