From c9859f36b6048a5f0d878fc2fae6fc48696a6d19 Mon Sep 17 00:00:00 2001 From: Adam Date: Sat, 22 Aug 2020 13:36:45 -0400 Subject: [PATCH] hiscore panel: fix lookup to run on edt It modifies various fields in the ui and should be running on edt. Additionally add a clear listener to reset the search state when the input is cleared. This requires changing the hiscore client to be async so that the response can be properly applied on the edt --- .../http/api/hiscore/HiscoreClient.java | 91 +++++++++++++------ .../client/plugins/hiscore/HiscorePanel.java | 80 ++++++++-------- .../plugins/hiscore/HiscorePanelTest.java | 3 +- 3 files changed, 109 insertions(+), 65 deletions(-) diff --git a/http-api/src/main/java/net/runelite/http/api/hiscore/HiscoreClient.java b/http-api/src/main/java/net/runelite/http/api/hiscore/HiscoreClient.java index 2807f8f411..cad7b00094 100644 --- a/http-api/src/main/java/net/runelite/http/api/hiscore/HiscoreClient.java +++ b/http-api/src/main/java/net/runelite/http/api/hiscore/HiscoreClient.java @@ -25,8 +25,11 @@ package net.runelite.http.api.hiscore; import java.io.IOException; +import java.util.concurrent.CompletableFuture; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import okhttp3.Call; +import okhttp3.Callback; import okhttp3.HttpUrl; import okhttp3.OkHttpClient; import okhttp3.Request; @@ -46,16 +49,14 @@ public class HiscoreClient return lookup(username, endpoint.getHiscoreURL()); } + public CompletableFuture lookupAsync(String username, HiscoreEndpoint endpoint) + { + return lookupAsync(username, endpoint.getHiscoreURL()); + } + public HiscoreResult lookup(String username, HttpUrl endpoint) throws IOException { - HiscoreResultBuilder resultBuilder = lookupUsername(username, endpoint); - - if (resultBuilder == null) - { - return null; - } - - return resultBuilder.build(); + return lookupSync(username, endpoint); } public HiscoreResult lookup(String username) throws IOException @@ -65,15 +66,13 @@ public class HiscoreClient public SingleHiscoreSkillResult lookup(String username, HiscoreSkill skill, HiscoreEndpoint endpoint) throws IOException { - HiscoreResultBuilder resultBuilder = lookupUsername(username, endpoint.getHiscoreURL()); + HiscoreResult result = lookupSync(username, endpoint.getHiscoreURL()); - if (resultBuilder == null) + if (result == null) { return null; } - HiscoreResult result = resultBuilder.build(); - Skill requested = result.getSkill(skill); SingleHiscoreSkillResult skillResult = new SingleHiscoreSkillResult(); skillResult.setPlayer(username); @@ -87,7 +86,44 @@ public class HiscoreClient return lookup(username, skill, HiscoreEndpoint.NORMAL); } - private HiscoreResultBuilder lookupUsername(String username, HttpUrl hiscoreUrl) throws IOException + private HiscoreResult lookupSync(String username, HttpUrl hiscoreUrl) throws IOException + { + try (Response response = client.newCall(buildRequest(username, hiscoreUrl)).execute()) + { + return processResponse(username, response); + } + } + + private CompletableFuture lookupAsync(String username, HttpUrl hiscoreUrl) + { + CompletableFuture future = new CompletableFuture<>(); + + client.newCall(buildRequest(username, hiscoreUrl)).enqueue(new Callback() + { + @Override + public void onFailure(Call call, IOException e) + { + future.completeExceptionally(e); + } + + @Override + public void onResponse(Call call, Response response) throws IOException + { + try + { + future.complete(processResponse(username, response)); + } + finally + { + response.close(); + } + } + }); + + return future; + } + + private static Request buildRequest(String username, HttpUrl hiscoreUrl) { HttpUrl url = hiscoreUrl.newBuilder() .addQueryParameter("player", username) @@ -95,28 +131,29 @@ public class HiscoreClient log.debug("Built URL {}", url); - Request okrequest = new Request.Builder() + return new Request.Builder() .url(url) .build(); + } - String responseStr; - - try (Response okresponse = client.newCall(okrequest).execute()) + private static HiscoreResult processResponse(String username, Response response) throws IOException + { + if (!response.isSuccessful()) { - if (!okresponse.isSuccessful()) + if (response.code() == 404) { - switch (okresponse.code()) - { - case 404: - return null; - default: - throw new IOException("Error retrieving data from Jagex Hiscores: " + okresponse); - } + return null; } - responseStr = okresponse.body().string(); + throw new IOException("Error retrieving data from Jagex Hiscores: " + response); } + String responseStr = response.body().string(); + return parseResponse(username, responseStr); + } + + private static HiscoreResult parseResponse(String username, String responseStr) throws IOException + { CSVParser parser = CSVParser.parse(responseStr, CSVFormat.DEFAULT); HiscoreResultBuilder hiscoreBuilder = new HiscoreResultBuilder(); @@ -147,6 +184,6 @@ public class HiscoreClient hiscoreBuilder.setNextSkill(skill); } - return hiscoreBuilder; + return hiscoreBuilder.build(); } } diff --git a/runelite-client/src/main/java/net/runelite/client/plugins/hiscore/HiscorePanel.java b/runelite-client/src/main/java/net/runelite/client/plugins/hiscore/HiscorePanel.java index 1843e50c7b..5d74ca37b2 100644 --- a/runelite-client/src/main/java/net/runelite/client/plugins/hiscore/HiscorePanel.java +++ b/runelite-client/src/main/java/net/runelite/client/plugins/hiscore/HiscorePanel.java @@ -38,17 +38,16 @@ import java.awt.event.KeyListener; import java.awt.event.MouseAdapter; import java.awt.event.MouseEvent; import java.awt.image.BufferedImage; -import java.io.IOException; import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.ScheduledExecutorService; import javax.annotation.Nullable; import javax.inject.Inject; import javax.swing.ImageIcon; import javax.swing.JLabel; import javax.swing.JPanel; +import javax.swing.SwingUtilities; import javax.swing.border.EmptyBorder; import lombok.extern.slf4j.Slf4j; import net.runelite.api.Client; @@ -119,7 +118,6 @@ public class HiscorePanel extends PluginPanel HiscoreEndpoint.NORMAL, HiscoreEndpoint.IRONMAN, HiscoreEndpoint.HARDCORE_IRONMAN, HiscoreEndpoint.ULTIMATE_IRONMAN, HiscoreEndpoint.DEADMAN, HiscoreEndpoint.TOURNAMENT }; - private final ScheduledExecutorService executor; private final Client client; private final HiscoreConfig config; private final NameAutocompleter nameAutocompleter; @@ -140,10 +138,9 @@ public class HiscorePanel extends PluginPanel private boolean loading = false; @Inject - public HiscorePanel(ScheduledExecutorService scheduledExecutorService, @Nullable Client client, + public HiscorePanel(@Nullable Client client, HiscoreConfig config, NameAutocompleter nameAutocompleter, OkHttpClient okHttpClient) { - this.executor = scheduledExecutorService; this.client = client; this.config = config; this.nameAutocompleter = nameAutocompleter; @@ -171,7 +168,7 @@ public class HiscorePanel extends PluginPanel searchBar.setBackground(ColorScheme.DARKER_GRAY_COLOR); searchBar.setHoverBackgroundColor(ColorScheme.DARK_GRAY_HOVER_COLOR); searchBar.setMinimumSize(new Dimension(0, 30)); - searchBar.addActionListener(e -> executor.execute(this::lookup)); + searchBar.addActionListener(e -> lookup()); searchBar.addMouseListener(new MouseAdapter() { @Override @@ -194,6 +191,12 @@ public class HiscorePanel extends PluginPanel } } }); + searchBar.addClearListener(() -> + { + searchBar.setIcon(IconTextField.Icon.SEARCH); + searchBar.setEditable(true); + loading = false; + }); add(searchBar, c); c.gridy++; @@ -231,7 +234,7 @@ public class HiscorePanel extends PluginPanel return; } - executor.execute(HiscorePanel.this::lookup); + lookup(); } }); @@ -359,14 +362,12 @@ public class HiscorePanel extends PluginPanel { searchBar.setText(username); resetEndpoints(); - executor.execute(this::lookup); + lookup(); } private void lookup() { - String lookup = searchBar.getText(); - - lookup = sanitize(lookup); + final String lookup = sanitize(searchBar.getText()); if (Strings.isNullOrEmpty(lookup)) { @@ -401,33 +402,40 @@ public class HiscorePanel extends PluginPanel selectedEndPoint = HiscoreEndpoint.NORMAL; } - HiscoreResult result; - try - { - log.debug("Hiscore endpoint " + selectedEndPoint.name() + " selected"); - result = hiscoreClient.lookup(lookup, selectedEndPoint); - } - catch (IOException ex) - { - log.warn("Error fetching Hiscore data " + ex.getMessage()); - searchBar.setIcon(IconTextField.Icon.ERROR); - searchBar.setEditable(true); - loading = false; - return; - } + hiscoreClient.lookupAsync(lookup, selectedEndPoint).whenCompleteAsync((result, ex) -> + SwingUtilities.invokeLater(() -> + { + if (!sanitize(searchBar.getText()).equals(lookup)) + { + // search has changed in the meantime + return; + } - if (result == null) - { - searchBar.setIcon(IconTextField.Icon.ERROR); - searchBar.setEditable(true); - loading = false; - return; - } + if (result == null || ex != null) + { + if (ex != null) + { + log.warn("Error fetching Hiscore data " + ex.getMessage()); + } - //successful player search - searchBar.setIcon(IconTextField.Icon.SEARCH); - searchBar.setEditable(true); - loading = false; + searchBar.setIcon(IconTextField.Icon.ERROR); + searchBar.setEditable(true); + loading = false; + return; + } + + //successful player search + searchBar.setIcon(IconTextField.Icon.SEARCH); + searchBar.setEditable(true); + loading = false; + + applyHiscoreResult(result); + })); + } + + private void applyHiscoreResult(HiscoreResult result) + { + assert SwingUtilities.isEventDispatchThread(); nameAutocompleter.addToSearchHistory(result.getPlayer().toLowerCase()); diff --git a/runelite-client/src/test/java/net/runelite/client/plugins/hiscore/HiscorePanelTest.java b/runelite-client/src/test/java/net/runelite/client/plugins/hiscore/HiscorePanelTest.java index ae2f63144b..e8cc6293cf 100644 --- a/runelite-client/src/test/java/net/runelite/client/plugins/hiscore/HiscorePanelTest.java +++ b/runelite-client/src/test/java/net/runelite/client/plugins/hiscore/HiscorePanelTest.java @@ -24,7 +24,6 @@ */ package net.runelite.client.plugins.hiscore; -import java.util.concurrent.ScheduledExecutorService; import static net.runelite.client.plugins.hiscore.HiscorePanel.formatLevel; import okhttp3.OkHttpClient; import static org.junit.Assert.assertEquals; @@ -36,7 +35,7 @@ public class HiscorePanelTest @Test public void testConstructor() { - new HiscorePanel(mock(ScheduledExecutorService.class), null, mock(HiscoreConfig.class), mock(NameAutocompleter.class), mock(OkHttpClient.class)); + new HiscorePanel(null, mock(HiscoreConfig.class), mock(NameAutocompleter.class), mock(OkHttpClient.class)); } @Test