From b823fe1ee0dcdee6c566ce99319966344e5def71 Mon Sep 17 00:00:00 2001 From: Austin Keener Date: Tue, 22 Feb 2022 21:48:27 -0500 Subject: [PATCH] Restructure message creation to handle a lack of guild_id (#2039) --- .../jda/api/entities/MessageReference.java | 3 +- .../dv8tion/jda/api/entities/NewsChannel.java | 2 +- .../jda/internal/entities/EntityBuilder.java | 117 ++++++++++-------- .../internal/entities/ReceivedMessage.java | 9 ++ .../internal/handle/MessageCreateHandler.java | 7 +- .../internal/handle/MessageUpdateHandler.java | 12 +- .../MessageContextInteractionImpl.java | 33 ++--- .../component/ComponentInteractionImpl.java | 11 +- 8 files changed, 108 insertions(+), 86 deletions(-) diff --git a/src/main/java/net/dv8tion/jda/api/entities/MessageReference.java b/src/main/java/net/dv8tion/jda/api/entities/MessageReference.java index 46f7f875d6..8722a1caca 100644 --- a/src/main/java/net/dv8tion/jda/api/entities/MessageReference.java +++ b/src/main/java/net/dv8tion/jda/api/entities/MessageReference.java @@ -154,7 +154,8 @@ public RestAction resolve(boolean update) Route.CompiledRoute route = Route.Messages.GET_MESSAGE.compile(getChannelId(), getMessageId()); return new RestActionImpl<>(jda, route, (response, request) -> { - Message created = jda.getEntityBuilder().createMessage(response.getObject(), getChannel(), false); + // channel can be null for MessageReferences, but we've already checked for that above, so it is nonnull here + Message created = jda.getEntityBuilder().createMessage(response.getObject(), channel, false); this.referencedMessage = created; return created; }); diff --git a/src/main/java/net/dv8tion/jda/api/entities/NewsChannel.java b/src/main/java/net/dv8tion/jda/api/entities/NewsChannel.java index 0a950f07f7..8d57072275 100644 --- a/src/main/java/net/dv8tion/jda/api/entities/NewsChannel.java +++ b/src/main/java/net/dv8tion/jda/api/entities/NewsChannel.java @@ -177,7 +177,7 @@ default RestAction crosspostMessageById(@Nonnull String messageId) throw new MissingAccessException(this, Permission.VIEW_CHANNEL); Route.CompiledRoute route = Route.Messages.CROSSPOST_MESSAGE.compile(getId(), messageId); return new RestActionImpl<>(getJDA(), route, - (response, request) -> request.getJDA().getEntityBuilder().createMessage(response.getObject())); + (response, request) -> request.getJDA().getEntityBuilder().createMessage(response.getObject(), this, false)); } /** diff --git a/src/main/java/net/dv8tion/jda/internal/entities/EntityBuilder.java b/src/main/java/net/dv8tion/jda/internal/entities/EntityBuilder.java index eca23f512e..383244bbaf 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/EntityBuilder.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/EntityBuilder.java @@ -60,6 +60,7 @@ import org.apache.commons.collections4.map.CaseInsensitiveMap; import org.slf4j.Logger; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.time.Instant; import java.time.OffsetDateTime; @@ -1327,72 +1328,78 @@ public Role createRole(GuildImpl guild, DataObject roleJson, long guildId) return role; } - public Message createMessage(DataObject jsonObject) { return createMessage(jsonObject, false); } - public Message createMessage(DataObject jsonObject, boolean modifyCache) + //If we aren't sure whether we have a PrivateChannel constructed yet when we want to build this method + public ReceivedMessage createMessagePrivateChannel(DataObject jsonObject, boolean modifyCache) { final long channelId = jsonObject.getLong("channel_id"); + final DataObject author = jsonObject.getObject("author"); + final long authorId = author.getLong("id"); - //TODO-v5-unified-channel-cache - MessageChannel chan = getJDA().getTextChannelById(channelId); - if (chan == null) - chan = getJDA().getNewsChannelById(channelId); - if (chan == null) - chan = getJDA().getPrivateChannelById(channelId); - if (chan == null) - chan = getJDA().getThreadChannelById(channelId); - if (chan == null && !jsonObject.isNull("guild_id")) - throw new IllegalArgumentException(MISSING_CHANNEL); - - return createMessage(jsonObject, chan, modifyCache); - } - public ReceivedMessage createMessage(DataObject jsonObject, @Nullable MessageChannel channel, boolean modifyCache) - { - long channelId = jsonObject.getUnsignedLong("channel_id"); - if (channel != null && channelId != channel.getIdLong()) + PrivateChannelImpl channel = (PrivateChannelImpl) getJDA().getPrivateChannelById(channelId); + boolean isAuthorSelfUser = authorId == getJDA().getSelfUser().getIdLong(); + if (channel == null) { - //TODO-v5-unified-channel-cache - channel = api.getTextChannelById(channelId); - if (channel == null) - channel = api.getNewsChannelById(channelId); - if (channel == null) - channel = api.getPrivateChannelById(channelId); + DataObject channelData = DataObject.empty() + .put("id", channelId); + + //if we see an author that isn't us, we can assume that is the other side of this private channel + //if the author is us, we learn no information about the user at the other end + if (!isAuthorSelfUser) + channelData.put("recipient", author); + + //even without knowing the user at the other end, we can still construct a minimal channel + channel = (PrivateChannelImpl) createPrivateChannel(channelData); } - final long id = jsonObject.getLong("id"); + return createMessagePrivateChannel(jsonObject, channel, modifyCache); + } + + //We definitely have a PrivateChannel constructed that we want to use, but make sure it has all the data + // it can be filled with before building the message + public ReceivedMessage createMessagePrivateChannel(DataObject jsonObject, @Nonnull PrivateChannel channel, boolean modifyCache) + { final DataObject author = jsonObject.getObject("author"); final long authorId = author.getLong("id"); - MemberImpl member = null; - if (jsonObject.isNull("guild_id")) + boolean isAuthorSelfUser = authorId == getJDA().getSelfUser().getIdLong(); + if (channel.getUser() == null && !isAuthorSelfUser) { - //we know it's a private channel - PrivateChannelImpl priv = (PrivateChannelImpl) channel; + //if we see an author that isn't us, we can assume that is the other side of this private channel + //if the author is us, we learn no information about the user at the other end + ((PrivateChannelImpl) channel).setUser(createUser(author)); + } - boolean isAuthorSelfUser = authorId == getJDA().getSelfUser().getIdLong(); - if (priv == null) - { - DataObject channelData = DataObject.empty() - .put("id", channelId); + return createMessage0(jsonObject, channel, modifyCache); + } + + //We aren't sure which guild channel to use or if it is even cached, so find it and use it to build a message or throw + public ReceivedMessage createMessageGuildChannel(DataObject jsonObject, boolean modifyCache) + { + final long channelId = jsonObject.getLong("channel_id"); - //if we see an author that isn't us, we can assume that is the other side of this private channel - //if the author is us, we learn no information about the user at the other end - if (!isAuthorSelfUser) - channelData.put("recipient", author); + GuildMessageChannel chan = getJDA().getChannelById(GuildMessageChannel.class, channelId); + if (chan == null) + throw new IllegalArgumentException(MISSING_CHANNEL); - //even without knowing the user at the other end, we can still construct a minimal channel - channel = createPrivateChannel(channelData); - } - else if (priv.getUser() == null && !isAuthorSelfUser) - { - //if we see an author that isn't us, we can assume that is the other side of this private channel - //if the author is us, we learn no information about the user at the other end - priv.setUser(createUser(author)); - } + return createMessage0(jsonObject, chan, modifyCache); + } - } + //Creation of messages _needs_ to go through either the GuildMessageChannel side or PrivateChannel to ensure that + // private channels get built/updated as needed and guild channels can throw MISSING_CHANNEL as needed. + public ReceivedMessage createMessage(DataObject jsonObject, @Nonnull MessageChannel channel, boolean modifyCache) + { + if (channel.getType().isGuild()) + return createMessage0(jsonObject, channel, modifyCache); + else + return createMessagePrivateChannel(jsonObject, (PrivateChannel) channel, modifyCache); + } - else if (channel == null) - throw new IllegalArgumentException(MISSING_CHANNEL); + private ReceivedMessage createMessage0(DataObject jsonObject, @Nonnull MessageChannel channel, boolean modifyCache) + { + final long id = jsonObject.getLong("id"); + final DataObject author = jsonObject.getObject("author"); + final long authorId = author.getLong("id"); + MemberImpl member = null; if (channel.getType().isGuild() && !jsonObject.isNull("member")) { @@ -1445,9 +1452,17 @@ else if (channel == null) else { //Assume private channel if (authorId == getJDA().getSelfUser().getIdLong()) + { user = getJDA().getSelfUser(); + } else + { + //Note, while PrivateChannel.getUser() can produce null, this invocation of it WILL NOT produce null + // because when the bot receives a message in a private channel that was _not authored by the bot_ then + // the message had to have come from the user, so that means that we had all the information to build + // the channel properly (or fill-in the missing user info of an existing partial channel) user = ((PrivateChannel) channel).getUser(); + } } if (modifyCache && !fromWebhook) // update the user information on message receive diff --git a/src/main/java/net/dv8tion/jda/internal/entities/ReceivedMessage.java b/src/main/java/net/dv8tion/jda/internal/entities/ReceivedMessage.java index 400ca3a2d7..05cc39689d 100644 --- a/src/main/java/net/dv8tion/jda/internal/entities/ReceivedMessage.java +++ b/src/main/java/net/dv8tion/jda/internal/entities/ReceivedMessage.java @@ -763,6 +763,15 @@ public TextChannel getTextChannel() return (TextChannel) channel; } + @Nonnull + @Override + public NewsChannel getNewsChannel() + { + if (!isFromType(ChannelType.NEWS)) + throw new IllegalStateException("This message was not sent in a news channel"); + return (NewsChannel) channel; + } + @Override public Category getCategory() { diff --git a/src/main/java/net/dv8tion/jda/internal/handle/MessageCreateHandler.java b/src/main/java/net/dv8tion/jda/internal/handle/MessageCreateHandler.java index 24fa5ba047..92689be083 100644 --- a/src/main/java/net/dv8tion/jda/internal/handle/MessageCreateHandler.java +++ b/src/main/java/net/dv8tion/jda/internal/handle/MessageCreateHandler.java @@ -45,7 +45,8 @@ protected Long handleInternally(DataObject content) } JDAImpl jda = getJDA(); - if (!content.isNull("guild_id")) + boolean isGuild = !content.isNull("guild_id"); + if (isGuild) { long guildId = content.getLong("guild_id"); if (jda.getGuildSetupController().isLocked(guildId)) @@ -55,7 +56,9 @@ protected Long handleInternally(DataObject content) Message message; try { - message = jda.getEntityBuilder().createMessage(content, true); + message = isGuild + ? jda.getEntityBuilder().createMessageGuildChannel(content, true) + : jda.getEntityBuilder().createMessagePrivateChannel(content, true); } catch (IllegalArgumentException e) { diff --git a/src/main/java/net/dv8tion/jda/internal/handle/MessageUpdateHandler.java b/src/main/java/net/dv8tion/jda/internal/handle/MessageUpdateHandler.java index 56ecebb7e6..434e1c97e2 100644 --- a/src/main/java/net/dv8tion/jda/internal/handle/MessageUpdateHandler.java +++ b/src/main/java/net/dv8tion/jda/internal/handle/MessageUpdateHandler.java @@ -37,7 +37,8 @@ public MessageUpdateHandler(JDAImpl api) @Override protected Long handleInternally(DataObject content) { - if (!content.isNull("guild_id")) + boolean isGuild = !content.isNull("guild_id"); + if (isGuild) { long guildId = content.getLong("guild_id"); if (getJDA().getGuildSetupController().isLocked(guildId)) @@ -51,7 +52,7 @@ protected Long handleInternally(DataObject content) { MessageType type = MessageType.fromId(content.getInt("type")); if (!type.isSystem()) - return handleMessage(content); + return handleMessage(content, isGuild); WebSocketClient.LOG.debug("JDA received a message update for an unexpected message type. Type: {} JSON: {}", type, content); return null; } @@ -67,12 +68,14 @@ else if (!content.isNull("embeds")) return null; } - private Long handleMessage(DataObject content) + private Long handleMessage(DataObject content, boolean isGuild) { Message message; try { - message = getJDA().getEntityBuilder().createMessage(content); + message = isGuild + ? getJDA().getEntityBuilder().createMessageGuildChannel(content, true) + : getJDA().getEntityBuilder().createMessagePrivateChannel(content, true); } catch (IllegalArgumentException e) { @@ -115,6 +118,7 @@ private Long handleMessageEmbed(DataObject content) LinkedList embeds = new LinkedList<>(); //TODO-v5-unified-channel-cache + //TODO-v5: handle for threads. MessageChannel channel = getJDA().getTextChannelsView().get(channelId); if (channel == null) channel = getJDA().getNewsChannelById(channelId); diff --git a/src/main/java/net/dv8tion/jda/internal/interactions/command/MessageContextInteractionImpl.java b/src/main/java/net/dv8tion/jda/internal/interactions/command/MessageContextInteractionImpl.java index b9a0fd72fd..ca8b5b208b 100644 --- a/src/main/java/net/dv8tion/jda/internal/interactions/command/MessageContextInteractionImpl.java +++ b/src/main/java/net/dv8tion/jda/internal/interactions/command/MessageContextInteractionImpl.java @@ -26,27 +26,7 @@ public class MessageContextInteractionImpl extends ContextInteractionImpl - { - //temporary fix to set the guild_id for messages that do not contain it - if (data.hasKey("guild_id")) - { - resolved.put("guild_id", data.getLong("guild_id")); - } - return parse(jda, resolved); - }); - } - - private static Message parse(JDAImpl api, DataObject resolved) - { - DataObject messages = resolved.getObject("messages"); - DataObject message = messages.getObject(messages.keys().iterator().next()); - //manually include the guild id - if (resolved.hasKey("guild_id")) - { - message.put("guild_id", resolved.getLong("guild_id")); - } - return api.getEntityBuilder().createMessage(message); + super(jda, data, resolved -> parse(jda, data, resolved)); } @Override @@ -54,4 +34,15 @@ public MessageChannel getChannel() { return (MessageChannel) super.getChannel(); } + + private static Message parse(JDAImpl api, DataObject interactionData, DataObject resolved) + { + DataObject messages = resolved.getObject("messages"); + DataObject message = messages.getObject(messages.keys().iterator().next()); + + //Hopefully in the future we can ask 'message.hasKey("guild_id")' instead. + return interactionData.hasKey("guild_id") + ? api.getEntityBuilder().createMessageGuildChannel(message, false) + : api.getEntityBuilder().createMessagePrivateChannel(message, false); + } } diff --git a/src/main/java/net/dv8tion/jda/internal/interactions/component/ComponentInteractionImpl.java b/src/main/java/net/dv8tion/jda/internal/interactions/component/ComponentInteractionImpl.java index d5dfec0f17..0b9722cf6d 100644 --- a/src/main/java/net/dv8tion/jda/internal/interactions/component/ComponentInteractionImpl.java +++ b/src/main/java/net/dv8tion/jda/internal/interactions/component/ComponentInteractionImpl.java @@ -42,12 +42,11 @@ public ComponentInteractionImpl(JDAImpl jda, DataObject data) DataObject messageJson = data.getObject("message"); messageId = messageJson.getUnsignedLong("id"); - if (data.hasKey("guild_id")) - { - messageJson.put("guild_id", data.getLong("guild_id")); - } - - message = messageJson.isNull("type") ? null : jda.getEntityBuilder().createMessage(messageJson); + message = messageJson.isNull("type") + ? null + : data.hasKey("guild_id") + ? jda.getEntityBuilder().createMessageGuildChannel(messageJson, false) + : jda.getEntityBuilder().createMessagePrivateChannel(messageJson, false); } @Nonnull