Fix #22794: Improve handling of MapRoulette server issues

Signed-off-by: Taylor Smock <tsmock@meta.com>

src/main/java/org/openstreetmap/josm/plugins/maproulette/MapRoulette.java

@@ -1,6 +1,8 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.maproulette;
 
+import java.io.IOException;
+
 import org.openstreetmap.josm.actions.UploadAction;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.MapFrame;
@@ -16,6 +18,7 @@
 import org.openstreetmap.josm.plugins.maproulette.gui.preferences.MapRoulettePreferences;
 import org.openstreetmap.josm.plugins.maproulette.io.upload.EarlyUploadHook;
 import org.openstreetmap.josm.plugins.maproulette.io.upload.LateUploadHook;
+import org.openstreetmap.josm.plugins.maproulette.util.ExceptionDialogUtil;
 
 /**
  * The POJO entry point
@@ -47,7 +50,11 @@ public void mapFrameInitialized(MapFrame oldFrame, MapFrame newFrame) {
             newFrame.addToggleDialog(new TaskListPanel());
         } else {
             for (var task : ModifiedObjects.getLockedTasks()) {
-                TaskAPI.release(task.id());
+                try {
+                    TaskAPI.release(task.id());
+                } catch (IOException e) {
+                    ExceptionDialogUtil.explainException(e);
+                }
             }
         }
     }

src/main/java/org/openstreetmap/josm/plugins/maproulette/api/BundleAPI.java

@@ -43,16 +43,15 @@ private BundleAPI() {
      *
      * @param bundle The bundle to create
      * @return The created bundle. This is <i>not</i> the same as the bundle sent in.
+     * @throws IOException if there was a problem communicating with the server
      */
     @Nonnull
-    public static TaskBundle createBundle(TaskBundle bundle) {
+    public static TaskBundle createBundle(TaskBundle bundle) throws IOException {
         final var client = post(getBaseUrl() + PATH, null); // fixme add body
         try {
             try (var inputStream = client.connect().getContent()) {
                 return parseBundle(inputStream);
             }
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }

src/main/java/org/openstreetmap/josm/plugins/maproulette/api/ChallengeAPI.java

@@ -10,7 +10,6 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.UncheckedIOException;
 import java.time.Instant;
 import java.util.Map;
 import java.util.TreeMap;
@@ -83,17 +82,16 @@ public static void updateChallengeArchive(long id, boolean isArchived) {
      * @param currentTask The current task
      * @param status      The task status to limit the response by
      * @return The next task
+     * @throws IOException if there was a problem communicating with the server
      */
     @Nonnull
-    public static Task nextTask(long challengeId, long currentTask, String... status) {
+    public static Task nextTask(long challengeId, long currentTask, String... status) throws IOException {
         final var client = get(getBaseUrl() + PATH + "/" + challengeId + "/nextTask/" + currentTask,
                 status.length > 0 ? Map.of("statusList", String.join(",", status)) : null);
         try {
             try (var inputStream = client.connect().getContent()) {
                 return (Task) TaskParser.parseTask(inputStream);
             }
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -106,17 +104,16 @@ public static Task nextTask(long challengeId, long currentTask, String... status
      * @param currentTask The current task
      * @param status      The task status to limit the response by
      * @return The previous task
+     * @throws IOException if there was a problem communicating with the server
      */
     @Nonnull
-    public static Task previousTask(long challengeId, long currentTask, String... status) {
+    public static Task previousTask(long challengeId, long currentTask, String... status) throws IOException {
         final var client = get(getBaseUrl() + PATH + "/" + challengeId + "/previousTask/" + currentTask,
                 status.length > 0 ? Map.of("statusList", String.join(",", status)) : null);
         try {
             try (var inputStream = client.connect().getContent()) {
                 return (Task) TaskParser.parseTask(inputStream);
             }
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -131,10 +128,11 @@ public static Task previousTask(long challengeId, long currentTask, String... st
      * @param limit        The number of prioritized tasks to get. If less than zero, one is used.
      * @param proximity    The current task
      * @return The next task
+     * @throws IOException if there was a problem communicating with the server
      */
     @Nonnull
     public static Task[] prioritizedTask(long challengeId, @Nullable String searchString, @Nullable String[] tags,
-            int limit, long proximity) {
+            int limit, long proximity) throws IOException {
         return taskCollectionEndpoints("/tasks/prioritizedTasks", challengeId, searchString, tags, limit, proximity);
     }
 
@@ -147,9 +145,10 @@ public static Task[] prioritizedTask(long challengeId, @Nullable String searchSt
      * @param limit        The number of prioritized tasks to get. If less than zero, one is used.
      * @param proximity    The current task
      * @return The next task
+     * @throws IOException if there was a problem communicating with the server
      */
     private static Task[] taskCollectionEndpoints(@Nonnull String path, long challengeId, @Nullable String searchString,
-            @Nullable String[] tags, int limit, long proximity) {
+            @Nullable String[] tags, int limit, long proximity) throws IOException {
         Map<String, String> query = new TreeMap<>();
         if (searchString != null && !searchString.isBlank()) {
             query.put("s", searchString);
@@ -168,8 +167,6 @@ private static Task[] taskCollectionEndpoints(@Nonnull String path, long challen
             try (var inputStream = client.connect().getContent()) {
                 return (Task[]) TaskParser.parseTask(inputStream);
             }
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -184,9 +181,10 @@ private static Task[] taskCollectionEndpoints(@Nonnull String path, long challen
      * @param limit        The number of prioritized tasks to get. If less than zero, one is used.
      * @param proximity    The current task
      * @return The next task
+     * @throws IOException if there was a problem communicating with the server
      */
     public static Task[] randomTask(long challengeId, @Nullable String searchString, @Nullable String[] tags, int limit,
-            long proximity) {
+            long proximity) throws IOException {
         return taskCollectionEndpoints("/tasks/prioritizedTasks", challengeId, searchString, tags, limit, proximity);
     }
 
@@ -200,9 +198,10 @@ public static Task[] randomTask(long challengeId, @Nullable String searchString,
      * @param proximity         id of task around which geographically closest tasks are desired
      *                          (note: this seems like it might be a bug in the API)
      * @return The tasks
+     * @throws IOException if there was a problem communicating with the server
      */
     public static Task[] tasksNearby(long challengeId, long proximityId, boolean excludeSelfLocked, int limit,
-            long proximity) {
+            long proximity) throws IOException {
         Map<String, String> query = new TreeMap<>();
         if (!excludeSelfLocked) {
             query.put("excludeSelfLocked", "true");
@@ -218,8 +217,6 @@ public static Task[] tasksNearby(long challengeId, long proximityId, boolean exc
             try (var inputStream = client.connect().getContent()) {
                 return (Task[]) TaskParser.parseTask(inputStream);
             }
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -230,15 +227,14 @@ public static Task[] tasksNearby(long challengeId, long proximityId, boolean exc
      *
      * @param challengeId The challenge to get
      * @return The challenge
+     * @throws IOException if there was a problem communicating with the server
      */
-    public static Challenge challenge(long challengeId) {
+    public static Challenge challenge(long challengeId) throws IOException {
         final var client = get(getBaseUrl() + PATH + "/" + challengeId, null);
         try {
             try (var inputstream = client.connect().getContent()) {
                 return parseChallenge(inputstream);
             }
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }

src/main/java/org/openstreetmap/josm/plugins/maproulette/api/ProjectAPI.java

@@ -4,7 +4,6 @@
 import static org.openstreetmap.josm.plugins.maproulette.config.MapRouletteConfig.getBaseUrl;
 
 import java.io.IOException;
-import java.io.UncheckedIOException;
 
 import javax.annotation.Nonnull;
 
@@ -30,14 +29,13 @@ private ProjectAPI() {
      *
      * @param id The project to get
      * @return The parsed project object
+     * @throws IOException if there was a problem communicating with the server
      */
     @Nonnull
-    public static Project get(long id) {
+    public static Project get(long id) throws IOException {
         final var client = HttpClientUtils.get(getBaseUrl() + PROJECT + "/" + id);
         try (var inputstream = client.connect().getContent()) {
             return ProjectParser.parse(inputstream);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }

src/main/java/org/openstreetmap/josm/plugins/maproulette/api/TaskAPI.java

@@ -5,7 +5,6 @@
 import static org.openstreetmap.josm.plugins.maproulette.util.HttpClientUtils.put;
 
 import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.Map;
@@ -59,6 +58,7 @@ private TaskAPI() {
      * @param includeGeometries include the geometries
      * @param includeTags       include the tags
      * @return The collection of tasks
+     * @throws IOException if there was a problem communicating with the server
      */
     public static ClusteredPoint[] box(double minLon, double minLat, double maxLon, double maxLat, int limit, int page,
             boolean excludeLocked, String sort, String order, boolean includeTotal, boolean includeGeometries,
@@ -102,13 +102,12 @@ public static ClusteredPoint[] box(double minLon, double minLat, double maxLon,
      *
      * @param task The task to get
      * @return The task for the id
+     * @throws IOException if there was a problem communicating with the server
      */
-    public static Task get(long task) {
+    public static Task get(long task) throws IOException {
         final var client = HttpClientUtils.get(getBaseUrl() + TASK + "/" + task);
         try (var inputstream = client.connect().getContent()) {
             return (Task) TaskParser.parseTask(inputstream);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -119,13 +118,12 @@ public static Task get(long task) {
      *
      * @param task The task to start
      * @return The updated task
+     * @throws IOException if there was a problem communicating with the server
      */
-    public static Task start(long task) {
+    public static Task start(long task) throws IOException {
         final var client = HttpClientUtils.get(getBaseUrl() + TASK + "/" + task + "/start");
         try (var inputstream = client.connect().getContent()) {
             return (Task) TaskParser.parseTask(inputstream);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -136,13 +134,12 @@ public static Task start(long task) {
      *
      * @param task The task to unlock
      * @return The unlocked task
+     * @throws IOException if there was a problem communicating with the server
      */
-    public static Task release(long task) {
+    public static Task release(long task) throws IOException {
         final var client = HttpClientUtils.get(getBaseUrl() + TASK + "/" + task + "/release");
         try (var inputstream = client.connect().getContent()) {
             return (Task) TaskParser.parseTask(inputstream);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -153,13 +150,12 @@ public static Task release(long task) {
      *
      * @param task The task to update
      * @return The updated task
+     * @throws IOException if there was a problem communicating with the server
      */
-    public static Task refreshLock(long task) {
+    public static Task refreshLock(long task) throws IOException {
         final var client = HttpClientUtils.get(getBaseUrl() + TASK + "/" + task + "/refreshLock");
         try (var inputstream = client.connect().getContent()) {
             return (Task) TaskParser.parseTask(inputstream);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -170,13 +166,12 @@ public static Task refreshLock(long task) {
      *
      * @param task the task to update the changeset for
      * @return The updated task
+     * @throws IOException if there was a problem communicating with the server
      */
-    public static Task changeset(long task) {
+    public static Task changeset(long task) throws IOException {
         final var client = put(getBaseUrl() + TASK + "/" + task + "/changeset", Collections.emptyMap());
         try (var inputstream = client.connect().getContent()) {
             return (Task) TaskParser.parseTask(inputstream);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }
@@ -192,9 +187,10 @@ public static Task changeset(long task) {
      * @param requestReview Request review (or not), overrides user settings
      * @param completionResponses The completion responses
      * @return {@code true} if the task update was successful
+     * @throws IOException if there was a problem communicating with the server
      */
     public static boolean updateStatus(long task, TaskStatus status, String comment, String tags, Boolean requestReview,
-            Map<String, Option> completionResponses) {
+            Map<String, Option> completionResponses) throws IOException {
         Map<String, String> query = new TreeMap<>();
         if (comment != null && !comment.isBlank()) {
             query.put("comment", comment);
@@ -224,8 +220,6 @@ public static boolean updateStatus(long task, TaskStatus status, String comment,
                 Logging.info(content);
             }
             return response.getResponseCode() == 204;
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         } finally {
             client.disconnect();
         }

src/main/java/org/openstreetmap/josm/plugins/maproulette/api/model/ClusteredPoint.java

@@ -1,6 +1,7 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.maproulette.api.model;
 
+import java.io.IOException;
 import java.time.Instant;
 
 import javax.annotation.Nonnull;
@@ -48,8 +49,9 @@ public record ClusteredPoint(long id, long owner, @Nonnull String ownerName, Str
      * Get the task for this point.
      *
      * @return The task.
+     * @throws IOException if there was a problem communicating with the server
      */
-    public Task task() {
+    public Task task() throws IOException {
         return TaskAPI.get(this.id());
     }
 

src/main/java/org/openstreetmap/josm/plugins/maproulette/api_caching/ChallengeCache.java

@@ -1,10 +1,13 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.maproulette.api_caching;
 
+import java.io.IOException;
+
 import org.apache.commons.jcs3.access.CacheAccess;
 import org.openstreetmap.josm.data.cache.JCSCacheManager;
 import org.openstreetmap.josm.plugins.maproulette.api.ChallengeAPI;
 import org.openstreetmap.josm.plugins.maproulette.api.model.Challenge;
+import org.openstreetmap.josm.tools.Logging;
 
 /**
  * A cache for challenge objects, which don't change often. Use this if you don't need the absolute freshest data.
@@ -29,17 +32,32 @@ private ChallengeCache() {
      * @return {@code true} if the challenge is hidden and its tasks should not be shown
      */
     public static boolean isHidden(long id) {
-        final var challenge = challenge(id);
-        return challenge.deleted();
+        try {
+            final var challenge = challenge(id);
+            return challenge.deleted();
+        } catch (IOException ioException) {
+            Logging.trace(ioException);
+            return false;
+        }
     }
 
     /**
      * Cache results from {@link ChallengeAPI#challenge(long)}
      *
      * @param id The challenge id
      * @return The cached challenge
+     * @throws IOException if there was a problem communicating with the server
      */
-    public static Challenge challenge(long id) {
-        return CACHE.get(id, () -> ChallengeAPI.challenge(id));
+    public static Challenge challenge(long id) throws IOException {
+        if (CACHE.get(id) != null) {
+            return CACHE.get(id);
+        }
+        synchronized (CACHE) {
+            if (CACHE.get(id) == null) {
+                final var challenge = ChallengeAPI.challenge(id);
+                CACHE.put(id, challenge);
+            }
+        }
+        return CACHE.get(id);
     }
 }