Change PrivateChannel#getUser to handle API nullability (#2012)

Co-authored-by: Florian Spieß <business@minn.dev>
Co-authored-by: Austin Keener <keeneraustin@yahoo.com>

src/main/java/net/dv8tion/jda/api/entities/PrivateChannel.java

@@ -15,7 +15,11 @@
  */
 package net.dv8tion.jda.api.entities;
 
+import net.dv8tion.jda.api.requests.RestAction;
+
+import javax.annotation.CheckReturnValue;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 /**
  * Represents the connection used for direct messaging.
@@ -27,8 +31,51 @@ public interface PrivateChannel extends MessageChannel
     /**
      * The {@link net.dv8tion.jda.api.entities.User User} that this {@link net.dv8tion.jda.api.entities.PrivateChannel PrivateChannel} communicates with.
      *
-     * @return A non-null {@link net.dv8tion.jda.api.entities.User User}.
+     * <p>This user is only null if this channel is currently uncached, and one the following occur:
+     * <ul>
+     *     <li>A reaction is removed</li>
+     *     <li>A reaction is added</li>
+     *     <li>A message is deleted</li>
+     *     <li>This account sends a message to a user from another shard (not shard 0)</li>
+     * </ul>
+     * The consequence of this is that for any message this bot receives from a guild or from other users, the user will not be null.
+     *
+     * <br>In order to retrieve a user that is null, use {@link #retrieveUser()}
+     *
+     * @return Possibly-null {@link net.dv8tion.jda.api.entities.User User}.
+     *
+     * @see #retrieveUser()
      */
-    @Nonnull
+    @Nullable
     User getUser();
+
+    /**
+     * Retrieves the {@link User User} that this {@link net.dv8tion.jda.api.entities.PrivateChannel PrivateChannel} communicates with.
+     *
+     * <br>This method fetches the channel from the API and retrieves the User from that.
+     *
+     * @return A {@link RestAction RestAction} to retrieve the {@link User User} that this {@link PrivateChannel PrivateChannel} communicates with.
+     */
+    @Nonnull
+    @CheckReturnValue
+    RestAction<User> retrieveUser();
+
+    /**
+     * The human-readable name of this channel.
+     *
+     * If getUser returns null, this method will return an empty String.
+     * This happens when JDA does not have enough information to populate the channel name.
+     *
+     * This will occur only when {@link #getUser()} is null, and the reasons are given in {@link #getUser()}
+     *
+     * If the channel name is important, {@link #retrieveUser()} should be used, instead.
+     *
+     * @return The name of this channel
+     *
+     * @see #retrieveUser()
+     * @see #getUser()
+     */
+    @Nonnull
+    @Override
+    String getName();
 }

src/main/java/net/dv8tion/jda/api/events/message/react/GenericMessageReactionEvent.java

@@ -175,6 +175,8 @@ public RestAction<Member> retrieveMember()
     {
         if (member != null)
             return new CompletedRestAction<>(getJDA(), member);
+        if (!getChannel().getType().isGuild())
+            throw new IllegalStateException("Cannot retrieve member for a private reaction not from a guild");
         return getGuild().retrieveMemberById(getUserIdLong());
     }
 

src/main/java/net/dv8tion/jda/api/events/message/react/MessageReactionAddEvent.java

@@ -40,7 +40,7 @@
  */
 public class MessageReactionAddEvent extends GenericMessageReactionEvent
 {
-    public MessageReactionAddEvent(@Nonnull JDA api, long responseNumber, @Nonnull User user,
+    public MessageReactionAddEvent(@Nonnull JDA api, long responseNumber, @Nullable User user,
                                    @Nullable Member member, @Nonnull MessageReaction reaction, long userId)
     {
         super(api, responseNumber, user, member, reaction, userId);

src/main/java/net/dv8tion/jda/internal/entities/EntityBuilder.java

@@ -1227,42 +1227,59 @@ public ThreadMember createThreadMember(ThreadChannelImpl threadChannel, Member m
 
     public PrivateChannel createPrivateChannel(DataObject json)
     {
-        final long channelId = json.getUnsignedLong("id");
-        PrivateChannel channel = api.getPrivateChannelById(channelId);
-        api.usedPrivateChannel(channelId);
-        if (channel != null)
-            return channel;
-
-        DataObject recipient = json.hasKey("recipients") ?
-            json.getArray("recipients").getObject(0) :
-            json.getObject("recipient");
-        final long userId = recipient.getLong("id");
-        UserImpl user = (UserImpl) getJDA().getUserById(userId);
-        if (user == null)
-        {   //The getJDA() can give us private channels connected to Users that we can no longer communicate with.
-            // As such, make a fake user and fake private channel.
-            user = createUser(recipient);
-        }
-
-        return createPrivateChannel(json, user);
+        return createPrivateChannel(json, null);
     }
 
     public PrivateChannel createPrivateChannel(DataObject json, UserImpl user)
     {
-        final long channelId = json.getLong("id");
-        PrivateChannelImpl priv = new PrivateChannelImpl(channelId, user)
-                .setLatestMessageIdLong(json.getLong("last_message_id", 0));
-        user.setPrivateChannel(priv);
+        final long channelId = json.getUnsignedLong("id");
+        PrivateChannelImpl channel = (PrivateChannelImpl) api.getPrivateChannelById(channelId);
+        if (channel == null)
+        {
+            channel = new PrivateChannelImpl(getJDA(), channelId, user)
+                    .setLatestMessageIdLong(json.getLong("last_message_id", 0));
+        }
+        UserImpl recipient = user;
+        if (channel.getUser() == null)
+        {
+            if (recipient == null && (json.hasKey("recipients") || json.hasKey("recipient")))
+            {
+                //if we don't know the recipient, and we have information on them, we can use that
+                DataObject recipientJson = json.hasKey("recipients") ?
+                        json.getArray("recipients").getObject(0) :
+                        json.getObject("recipient");
+                final long userId = recipientJson.getUnsignedLong("id");
+                recipient = (UserImpl) getJDA().getUserById(userId);
+                if (recipient == null)
+                {
+                    recipient = createUser(recipientJson);
+                }
+            }
+            if (recipient != null)
+            {
+                //update the channel if we have found the user
+                channel.setUser(user);
+            }
+        }
+        if (recipient != null)
+        {
+            recipient.setPrivateChannel(channel);
+        }
+        // only add channels to the cache when they come from an event, otherwise we would never remove the channel
+        cachePrivateChannel(channel);
+        api.usedPrivateChannel(channelId);
+        return channel;
+    }
 
-        // only add channels to cache when they come from an event, otherwise we would never remove the channel
+    private void cachePrivateChannel(PrivateChannelImpl priv)
+    {
         SnowflakeCacheViewImpl<PrivateChannel> privateView = getJDA().getPrivateChannelsView();
         try (UnlockHook hook = privateView.writeLock())
         {
-            privateView.getMap().put(channelId, priv);
+            privateView.getMap().put(priv.getIdLong(), priv);
         }
-        api.usedPrivateChannel(channelId);
-        getJDA().getEventCache().playbackCache(EventCache.Type.CHANNEL, channelId);
-        return priv;
+        api.usedPrivateChannel(priv.getIdLong());
+        getJDA().getEventCache().playbackCache(EventCache.Type.CHANNEL, priv.getIdLong());
     }
 
     @Nullable
@@ -1387,11 +1404,15 @@ public ReceivedMessage createMessage(DataObject jsonObject, @Nullable MessageCha
         final long authorId = author.getLong("id");
         MemberImpl member = null;
 
-        if (channel == null && jsonObject.isNull("guild_id") && authorId != getJDA().getSelfUser().getIdLong())
+        if (channel == null && jsonObject.isNull("guild_id"))
         {
             DataObject channelData = DataObject.empty()
-                    .put("id", channelId)
-                    .put("recipient", author);
+                    .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 (authorId != getJDA().getSelfUser().getIdLong())
+                    channelData.put("recipient", author);
+            //even without knowing the user at the other end, we can still construct a minimal channel
             channel = createPrivateChannel(channelData);
         }
         else if (channel == null)

src/main/java/net/dv8tion/jda/internal/entities/PrivateChannelImpl.java

@@ -16,26 +16,28 @@
 
 package net.dv8tion.jda.internal.entities;
 
-import net.dv8tion.jda.api.AccountType;
 import net.dv8tion.jda.api.JDA;
 import net.dv8tion.jda.api.entities.ChannelType;
 import net.dv8tion.jda.api.entities.PrivateChannel;
 import net.dv8tion.jda.api.entities.User;
 import net.dv8tion.jda.api.requests.RestAction;
+import net.dv8tion.jda.internal.JDAImpl;
 import net.dv8tion.jda.internal.entities.mixin.channel.middleman.MessageChannelMixin;
+import net.dv8tion.jda.internal.requests.CompletedRestAction;
 import net.dv8tion.jda.internal.requests.RestActionImpl;
 import net.dv8tion.jda.internal.requests.Route;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 public class PrivateChannelImpl extends AbstractChannelImpl<PrivateChannelImpl> implements PrivateChannel, MessageChannelMixin<PrivateChannelImpl>
 {
     private User user;
     private long latestMessageId;
 
-    public PrivateChannelImpl(long id, User user)
+    public PrivateChannelImpl(JDA api, long id, @Nullable User user)
     {
-        super(id, user.getJDA());
+        super(id, api);
         this.user = user;
     }
 
@@ -46,7 +48,7 @@ public ChannelType getType()
         return ChannelType.PRIVATE;
     }
 
-    @Nonnull
+    @Nullable
     @Override
     public User getUser()
     {
@@ -56,16 +58,34 @@ public User getUser()
 
     @Nonnull
     @Override
-    public String getName()
+    public RestAction<User> retrieveUser()
     {
-        return getUser().getName();
+        User user = getUser();
+        if (user != null)
+            return new CompletedRestAction<>(getJDA(), user);
+        //even if the user blocks the bot, this does not fail.
+        return retrievePrivateChannel()
+                .map(PrivateChannel::getUser);
     }
 
     @Nonnull
     @Override
-    public JDA getJDA()
+    public String getName()
     {
-        return user.getJDA();
+        User user = getUser();
+        if (user == null)
+        {
+            //don't break or override the contract of @NonNull
+            return "";
+        }
+        return user.getName();
+    }
+
+    @Nonnull
+    private RestAction<PrivateChannel> retrievePrivateChannel()
+    {
+        Route.CompiledRoute route = Route.Channels.GET_CHANNEL.compile(getId());
+        return new RestActionImpl<>(getJDA(), route, (response, request) -> ((JDAImpl) getJDA()).getEntityBuilder().createPrivateChannel(response.getObject()));
     }
 
     @Nonnull
@@ -85,7 +105,12 @@ public long getLatestMessageIdLong()
     @Override
     public boolean canTalk()
     {
-        return !user.isBot();
+        //The only way user is null is when an event is dispatched that doesn't give us enough information to build the recipient user,
+        // which only happens if this bot sends a message (or otherwise triggers an event) from a shard other than shard 0.
+        // The event will be received on shard 0 and not have enough information to build the recipient user.
+        //As such, since events will only happen in this channel if it is between the bot and the user, a null user is a valid channel state. 
+        // Events cannot happen between a bot and another bot, so the user would never be null in that case.
+        return user == null || !user.isBot();
     }
 
     @Override
@@ -120,6 +145,11 @@ public boolean canDeleteOtherUsersMessages()
         return false;
     }
 
+    public void setUser(User user)
+    {
+        this.user = user;
+    }
+
     @Override
     public PrivateChannelImpl setLatestMessageIdLong(long latestMessageId)
     {
@@ -149,7 +179,7 @@ public boolean equals(Object obj)
     @Override
     public String toString()
     {
-        return "PC:" + getUser().getName() + '(' + getId() + ')';
+        return "PC:" + getName() + '(' + getId() + ')';
     }
 
     private void updateUser()
@@ -162,7 +192,12 @@ private void updateUser()
 
     private void checkBot()
     {
-        if (getUser().isBot() && getJDA().getAccountType() == AccountType.BOT)
+        //The only way user is null is when an event is dispatched that doesn't give us enough information to build the recipient user,
+        // which only happens if this bot sends a message (or otherwise triggers an event) from a shard other than shard 0.
+        // The event will be received on shard 0 and not have enough information to build the recipient user.
+        //As such, since events will only happen in this channel if it is between the bot and the user, a null user is a valid channel state. 
+        // Events cannot happen between a bot and another bot, so the user would never be null in that case.
+        if (getUser() != null && getUser().isBot())
             throw new UnsupportedOperationException("Cannot send a private message between bots.");
     }
 }

src/main/java/net/dv8tion/jda/internal/entities/UserImpl.java

@@ -42,7 +42,7 @@ public class UserImpl extends UserById implements User
     protected String name;
     protected String avatarId;
     protected Profile profile;
-    protected long privateChannel = 0L;
+    protected long privateChannelId = 0L;
     protected boolean bot;
     protected boolean system;
     protected boolean fake = false;
@@ -113,7 +113,7 @@ public String getAsTag()
     @Override
     public boolean hasPrivateChannel()
     {
-        return privateChannel != 0;
+        return privateChannelId != 0;
     }
 
     @Nonnull
@@ -126,7 +126,7 @@ public RestAction<PrivateChannel> openPrivateChannel()
             return new RestActionImpl<>(getJDA(), route, body, (response, request) ->
             {
                 PrivateChannel priv = api.getEntityBuilder().createPrivateChannel(response.getObject(), this);
-                UserImpl.this.privateChannel = priv.getIdLong();
+                UserImpl.this.privateChannelId = priv.getIdLong();
                 return priv;
             });
         });
@@ -136,8 +136,8 @@ public PrivateChannel getPrivateChannel()
     {
         if (!hasPrivateChannel())
             return null;
-        PrivateChannel channel = getJDA().getPrivateChannelById(privateChannel);
-        return channel != null ? channel : new PrivateChannelImpl(privateChannel, this);
+        PrivateChannel channel = getJDA().getPrivateChannelById(privateChannelId);
+        return channel != null ? channel : new PrivateChannelImpl(getJDA(), privateChannelId, this);
     }
 
     @Nonnull
@@ -214,7 +214,7 @@ public UserImpl setProfile(Profile profile)
     public UserImpl setPrivateChannel(PrivateChannel privateChannel)
     {
         if (privateChannel != null)
-            this.privateChannel = privateChannel.getIdLong();
+            this.privateChannelId = privateChannel.getIdLong();
         return this;
     }