[Bugfix] retrieving public archived threads (#2115)

Co-authored-by: Austin Keener <keeneraustin@yahoo.com>
Co-authored-by: freya02 <41875020+freya022@users.noreply.github.com>

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

@@ -161,18 +161,6 @@ private void createGuildEmotePass(GuildImpl guildObj, DataArray array)
         }
     }
 
-    public TLongObjectMap<DataObject> convertToUserMap(ToLongFunction<DataObject> getId, DataArray array)
-    {
-        TLongObjectMap<DataObject> map = new TLongObjectHashMap<>();
-        for (int i = 0; i < array.length(); i++)
-        {
-            DataObject obj = array.getObject(i);
-            long userId = getId.applyAsLong(obj);
-            map.put(userId, obj);
-        }
-        return map;
-    }
-
     public GuildImpl createGuild(long guildId, DataObject guildJson, TLongObjectMap<DataObject> members, int memberCount)
     {
         final GuildImpl guildObj = new GuildImpl(getJDA(), guildId);
@@ -260,8 +248,8 @@ public GuildImpl createGuild(long guildId, DataObject guildJson, TLongObjectMap<
             createGuildChannel(guildObj, channelJson);
         }
 
-        TLongObjectMap<DataObject> voiceStates = convertToUserMap((o) -> o.getUnsignedLong("user_id", 0L), voiceStateArray);
-        TLongObjectMap<DataObject> presences = presencesArray.map(o1 -> convertToUserMap(o2 -> o2.getObject("user").getUnsignedLong("id"), o1)).orElseGet(TLongObjectHashMap::new);
+        TLongObjectMap<DataObject> voiceStates = Helpers.convertToMap((o) -> o.getUnsignedLong("user_id", 0L), voiceStateArray);
+        TLongObjectMap<DataObject> presences = presencesArray.map(o1 -> Helpers.convertToMap(o2 -> o2.getObject("user").getUnsignedLong("id"), o1)).orElseGet(TLongObjectHashMap::new);
         try (UnlockHook h1 = guildObj.getMembersView().writeLock();
              UnlockHook h2 = getJDA().getUsersView().writeLock())
         {

src/main/java/net/dv8tion/jda/internal/handle/GuildMembersChunkHandler.java

@@ -25,6 +25,7 @@
 import net.dv8tion.jda.internal.entities.GuildImpl;
 import net.dv8tion.jda.internal.entities.MemberImpl;
 import net.dv8tion.jda.internal.requests.WebSocketClient;
+import net.dv8tion.jda.internal.utils.Helpers;
 
 public class GuildMembersChunkHandler extends SocketHandler
 {
@@ -48,7 +49,7 @@ protected Long handleInternally(DataObject content)
             // Chunk handling
             EntityBuilder builder = getJDA().getEntityBuilder();
             TLongObjectMap<DataObject> presences = content.optArray("presences").map(it ->
-                builder.convertToUserMap(o -> o.getObject("user").getUnsignedLong("id"), it)
+                Helpers.convertToMap(o -> o.getObject("user").getUnsignedLong("id"), it)
             ).orElseGet(TLongObjectHashMap::new);
             for (int i = 0; i < members.length(); i++)
             {

src/main/java/net/dv8tion/jda/internal/requests/MemberChunkManager.java

@@ -25,6 +25,7 @@
 import net.dv8tion.jda.internal.entities.EntityBuilder;
 import net.dv8tion.jda.internal.entities.GuildImpl;
 import net.dv8tion.jda.internal.entities.MemberImpl;
+import net.dv8tion.jda.internal.utils.Helpers;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -193,7 +194,7 @@ private List<Member> toMembers(DataObject chunk)
             EntityBuilder builder = guild.getJDA().getEntityBuilder();
             DataArray memberArray = chunk.getArray("members");
             TLongObjectMap<DataObject> presences = chunk.optArray("presences").map(it ->
-                builder.convertToUserMap(o -> o.getObject("user").getUnsignedLong("id"), it)
+                Helpers.convertToMap(o -> o.getObject("user").getUnsignedLong("id"), it)
             ).orElseGet(TLongObjectHashMap::new);
             List<Member> collect = new ArrayList<>(memberArray.length());
             for (int i = 0; i < memberArray.length(); i++)

src/main/java/net/dv8tion/jda/internal/requests/restaction/pagination/PaginationActionImpl.java

@@ -34,8 +34,8 @@
 import java.util.function.Consumer;
 
 public abstract class PaginationActionImpl<T, M extends PaginationAction<T, M>>
-    extends RestActionImpl<List<T>>
-    implements PaginationAction<T, M>
+        extends RestActionImpl<List<T>>
+        implements PaginationAction<T, M>
 {
     protected final List<T> cached = new CopyOnWriteArrayList<>();
     protected final int maxLimit;
@@ -245,7 +245,8 @@ public final int getLimit()
     @Override
     public CompletableFuture<List<T>> takeAsync(int amount)
     {
-        return takeAsync0(amount, (task, list) -> forEachAsync(val -> {
+        return takeAsync0(amount, (task, list) -> forEachAsync(val ->
+        {
             list.add(val);
             return list.size() < amount;
         }, task::completeExceptionally));
@@ -255,7 +256,8 @@ public CompletableFuture<List<T>> takeAsync(int amount)
     @Override
     public CompletableFuture<List<T>> takeRemainingAsync(int amount)
     {
-        return takeAsync0(amount, (task, list) -> forEachRemainingAsync(val -> {
+        return takeAsync0(amount, (task, list) -> forEachRemainingAsync(val ->
+        {
             list.add(val);
             return list.size() < amount;
         }, task::completeExceptionally));
@@ -347,20 +349,39 @@ public void forEachRemaining(@Nonnull final Procedure<? super T> action)
         }
     }
 
+    // Introduced for paginating archived threads, because two endpoints require a different request parameter value format.
+    // May become more useful if discord introduces more pagination endpoints not using ids.
+    // Check ThreadChannelPaginationActionImpl
+    // Background of #getPaginationLastEvaluatedKey:
+    //     Archived thread channel pagination (example: TextChannel#retrieveArchivedPublicThreadChannels) would throw an exception,
+    //     where Discord complained about receiving a snowflake instead of an ISO8601 date.
+    //     The snowflake originated from this class (creating initial & subsequent requests),
+    //     while the correct value was set in ThreadChannelPaginationActionImpl for the initial request
+    //     and appended as a second value for subsequent requests.
+    //     However, withQueryParams is a simple string append and Discord only reads the first parameter.
+    //     If you debugged, you would see some duplicated fields on the compiled route.
+    //     The fix here is to let the implementor supply the "last" string value, which could be anything,
+    //     while the default implementation still would provide snowflakes
+    @Nonnull
+    protected String getPaginationLastEvaluatedKey(long lastId, T last)
+    {
+        return Long.toUnsignedString(lastId);
+    }
+
     @Override
     protected Route.CompiledRoute finalizeRoute()
     {
         Route.CompiledRoute route = super.finalizeRoute();
 
         final String limit = String.valueOf(this.getLimit());
-        final long last = this.lastKey;
+        final long localLastKey = this.lastKey;
 
         route = route.withQueryParams("limit", limit);
 
-        if (last != 0)
-            route = route.withQueryParams(order.getKey(), Long.toUnsignedString(last));
+        if (localLastKey != 0)
+            route = route.withQueryParams(order.getKey(), getPaginationLastEvaluatedKey(localLastKey, this.last));
         else if (order == PaginationOrder.FORWARD)
-            route = route.withQueryParams("after", "0");
+            route = route.withQueryParams("after", getPaginationLastEvaluatedKey(0, this.last));
 
         return route;
     }

src/main/java/net/dv8tion/jda/internal/requests/restaction/pagination/ThreadChannelPaginationActionImpl.java

@@ -9,20 +9,26 @@
 import net.dv8tion.jda.api.requests.Request;
 import net.dv8tion.jda.api.requests.Response;
 import net.dv8tion.jda.api.requests.restaction.pagination.ThreadChannelPaginationAction;
-import net.dv8tion.jda.api.utils.TimeUtil;
 import net.dv8tion.jda.api.utils.data.DataArray;
 import net.dv8tion.jda.api.utils.data.DataObject;
 import net.dv8tion.jda.internal.entities.EntityBuilder;
 import net.dv8tion.jda.internal.requests.Route;
+import net.dv8tion.jda.internal.utils.Helpers;
 
 import javax.annotation.Nonnull;
+import java.time.OffsetDateTime;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
 import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.List;
 
 public class ThreadChannelPaginationActionImpl extends PaginationActionImpl<ThreadChannel, ThreadChannelPaginationAction> implements ThreadChannelPaginationAction
 {
     protected final IThreadContainer channel;
+
+    // Whether IDs or ISO8601 timestamps shall be provided for all pagination requests.
+    // Some thread pagination endpoints require this odd and singular behavior throughout the discord api.
     protected final boolean useID;
 
     public ThreadChannelPaginationActionImpl(JDA api, Route.CompiledRoute route, IThreadContainer channel, boolean useID)
@@ -46,23 +52,27 @@ public EnumSet<PaginationOrder> getSupportedOrders()
         return EnumSet.of(PaginationOrder.BACKWARD);
     }
 
+    //Thread pagination supplies ISO8601 timestamps for some cases, see constructor
+    @Nonnull
     @Override
-    protected Route.CompiledRoute finalizeRoute()
+    protected String getPaginationLastEvaluatedKey(long lastId, ThreadChannel last)
     {
-        Route.CompiledRoute route = super.finalizeRoute();
-
-        final String limit = String.valueOf(this.limit.get());
-        final long last = this.lastKey;
+        if (useID)
+            return Long.toUnsignedString(lastId);
 
-        route = route.withQueryParams("limit", limit);
+        if (order == PaginationOrder.FORWARD && lastId == 0)
+        {
+            // first second of 2015 aka discords epoch, hard coding something older makes no sense to me
+            return "2015-01-01T00:00:00.000";
+        }
 
-        if (last == 0)
-            return route;
+        // this should be redundant, due to calling this with PaginationAction#getLast() as last param,
+        // but let's have this here.
+        if (last == null)
+            return OffsetDateTime.now(ZoneOffset.UTC).toString();
 
-        if (useID)
-            return route.withQueryParams("before", Long.toUnsignedString(last));
         // OffsetDateTime#toString() is defined to be ISO8601, needs no helper method.
-        return route.withQueryParams("before", TimeUtil.getTimeCreated(last).toString());
+        return last.getTimeArchiveInfoLastModified().toString();
     }
 
     @Override
@@ -75,14 +85,7 @@ protected void handleSuccess(Response response, Request<List<ThreadChannel>> req
         List<ThreadChannel> list = new ArrayList<>(threads.length());
         EntityBuilder builder = api.getEntityBuilder();
 
-        TLongObjectMap<DataObject> selfThreadMemberMap = new TLongObjectHashMap<>();
-        for (int i = 0; i < selfThreadMembers.length(); i++)
-        {
-            DataObject selfThreadMember = selfThreadMembers.getObject(i);
-
-            //Store the thread member based on the "id" which is the _thread's_ id, not the member's id (which would be our id)
-            selfThreadMemberMap.put(selfThreadMember.getLong("id"), selfThreadMember);
-        }
+        TLongObjectMap<DataObject> selfThreadMemberMap = Helpers.convertToMap((o) -> o.getUnsignedLong("id"), selfThreadMembers);
 
         for (int i = 0; i < threads.length(); i++)
         {

src/main/java/net/dv8tion/jda/internal/utils/Helpers.java

@@ -16,12 +16,18 @@
 
 package net.dv8tion.jda.internal.utils;
 
+import gnu.trove.map.TLongObjectMap;
+import gnu.trove.map.hash.TLongObjectHashMap;
+import net.dv8tion.jda.api.utils.data.DataArray;
+import net.dv8tion.jda.api.utils.data.DataObject;
+
 import javax.annotation.Nullable;
 import java.time.*;
 import java.time.format.DateTimeFormatter;
 import java.time.temporal.TemporalAccessor;
 import java.util.*;
 import java.util.function.Consumer;
+import java.util.function.ToLongFunction;
 
 /**
  * This class has major inspiration from <a href="https://commons.apache.org/proper/commons-lang/" target="_blank">Lang 3</a>
@@ -225,6 +231,18 @@ public static <E extends Enum<E>> EnumSet<E> copyEnumSet(Class<E> clazz, Collect
         return col == null || col.isEmpty() ? EnumSet.noneOf(clazz) : EnumSet.copyOf(col);
     }
 
+    public static TLongObjectMap<DataObject> convertToMap(ToLongFunction<DataObject> getId, DataArray array)
+    {
+        TLongObjectMap<DataObject> map = new TLongObjectHashMap<>();
+        for (int i = 0; i < array.length(); i++)
+        {
+            DataObject obj = array.getObject(i);
+            long objId = getId.applyAsLong(obj);
+            map.put(objId, obj);
+        }
+        return map;
+    }
+
     // ## ExceptionUtils ##
 
     public static <T extends Throwable> T appendCause(T throwable, Throwable cause)