Avoid using worker thread for getting sources -- this can block data downloads

tsmock · tsmock · commit 225abbb68525 · 2023-11-29T11:30:16.000-07:00
Signed-off-by: Taylor Smock &lt;tsmock@meta.com&gt;
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/MapWithAIPlugin.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/MapWithAIPlugin.java
@@ -3,15 +3,15 @@
 
 import static org.openstreetmap.josm.tools.I18n.tr;
 
-import javax.swing.JMenuItem;
-
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.swing.JMenuItem;
+
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.actions.PreferencesAction;
 import org.openstreetmap.josm.data.validation.OsmValidator;
@@ -29,6 +29,7 @@
 import org.openstreetmap.josm.plugins.PluginInformation;
 import org.openstreetmap.josm.plugins.mapwithai.backend.DownloadListener;
 import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAIAction;
+import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAIDataUtils;
 import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAILayer;
 import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAIMoveAction;
 import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAIObject;
@@ -135,7 +136,8 @@ public MapWithAIPlugin(PluginInformation info) {
         MainApplication.worker.execute(() -> UpdateProd.doProd(info.mainversion));
         // Preload the MapWithAILayerInfo for the JOSM download window
         // This reduces the amount of time taken for first button click by 100ms.
-        MainApplication.worker.execute(MapWithAILayerInfo::getInstance);
+        // Don't use the worker thread to avoid blocking user downloads
+        MapWithAIDataUtils.getForkJoinPool().execute(MapWithAILayerInfo::getInstance);
 
         destroyables.add(new MapWithAICopyProhibit());
     }
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnable.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnable.java
@@ -275,7 +275,7 @@ private static void realCleanup(DataSet dataSet, Bounds bounds, MapWithAIInfo in
         }
         (boundsToUse.isCollapsed() || boundsToUse.isOutOfTheWorld() ? dataSet.getWays()
                 : dataSet.searchWays(boundsToUse.toBBox())).stream().filter(way -> !way.isDeleted())
-                .forEach(GetDataRunnable::cleanupArtifacts);
+                        .forEach(GetDataRunnable::cleanupArtifacts);
     }
 
     /**
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAILayer.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAILayer.java
@@ -320,21 +320,21 @@ public static void createBadDataNotification() {
     private record OsmComparator(
             Collection<OsmPrimitive> previousSelection) implements Comparator<OsmPrimitive>, Serializable {
 
-        @Override
-        public int compare(OsmPrimitive o1, OsmPrimitive o2) {
-            if (previousSelection.contains(o1) == previousSelection.contains(o2)) {
-                if (o1.isTagged() == o2.isTagged()) {
-                    return o1.compareTo(o2);
-                } else if (o1.isTagged()) {
-                    return -1;
-                }
-                return 1;
-            }
-            if (previousSelection.contains(o1)) {
+    @Override
+    public int compare(OsmPrimitive o1, OsmPrimitive o2) {
+        if (previousSelection.contains(o1) == previousSelection.contains(o2)) {
+            if (o1.isTagged() == o2.isTagged()) {
+                return o1.compareTo(o2);
+            } else if (o1.isTagged()) {
                 return -1;
             }
             return 1;
         }
+        if (previousSelection.contains(o1)) {
+            return -1;
+        }
+        return 1;
+    }
 
     }
 
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWays.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWays.java
@@ -165,8 +165,8 @@ public static void filterDataSet(@Nonnull DataSet dataSet, @Nonnull List<Command
                 .collect(Collectors.toCollection(ArrayList::new));
         for (var i = 0; i < ways.size(); i++) {
             final var way1 = ways.get(i);
-            final var nearbyWays = dataSet.searchWays(way1.getBBox()).stream()
-                    .filter(MergeDuplicateWays::nonDeletedWay).filter(w -> !Objects.equals(w, way1)).toList();
+            final var nearbyWays = dataSet.searchWays(way1.getBBox()).stream().filter(MergeDuplicateWays::nonDeletedWay)
+                    .filter(w -> !Objects.equals(w, way1)).toList();
             for (final Way way2 : nearbyWays) {
                 final var command = checkForDuplicateWays(way1, way2);
                 final var deletedWays = new ArrayList<OsmPrimitive>();
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAICategory.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAICategory.java
@@ -28,12 +28,12 @@
  */
 public enum MapWithAICategory implements ISourceCategory<MapWithAICategory> {
 
-    BUILDING("data/closedway", "buildings", marktr("Buildings")),
-    HIGHWAY("presets/transport/way/way_road", "highways", marktr("Roads")),
-    ADDRESS("presets/misc/housenumber_small", "addresses", marktr("Addresses")),
-    POWER("presets/power/pole", "pole", marktr("Power")), PRESET("dialogs/search", "presets", marktr("Presets")),
-    FEATURED("presets/service/network-wireless", "featured", marktr("Featured")),
-    PREVIEW("presets/misc/fixme", "preview", marktr("Preview")), OTHER(null, "other", marktr("Other"));
+    BUILDING("data/closedway", "buildings", marktr("Buildings")), HIGHWAY("presets/transport/way/way_road", "highways",
+            marktr("Roads")), ADDRESS("presets/misc/housenumber_small", "addresses",
+                    marktr("Addresses")), POWER("presets/power/pole", "pole", marktr("Power")), PRESET("dialogs/search",
+                            "presets", marktr("Presets")), FEATURED("presets/service/network-wireless", "featured",
+                                    marktr("Featured")), PREVIEW("presets/misc/fixme", "preview",
+                                            marktr("Preview")), OTHER(null, "other", marktr("Other"));
 
     private static final Map<ImageSizes, Map<MapWithAICategory, ImageIcon>> iconCache = Collections
             .synchronizedMap(new EnumMap<>(ImageSizes.class));
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAILayerInfo.java
@@ -283,7 +283,7 @@ public List<MapWithAIInfo> compute() {
             }
             // This is literally to avoid allocations on startup
             final Preferences preferences;
-            if (Config.getPref() instanceof Preferences pref) {
+            if (Config.getPref()instanceof Preferences pref) {
                 preferences = pref;
             } else {
                 preferences = null;
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIType.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/mapwithai/MapWithAIType.java
@@ -11,8 +11,8 @@
  * @author Taylor Smock
  */
 public enum MapWithAIType implements ISourceType<MapWithAIType> {
-    FACEBOOK("facebook"), THIRD_PARTY("thirdParty"), ESRI("esri"), ESRI_FEATURE_SERVER("esriFeatureServer"),
-    MAPBOX_VECTOR_TILE("mvt"), PMTILES("pmtiles");
+    FACEBOOK("facebook"), THIRD_PARTY("thirdParty"), ESRI("esri"), ESRI_FEATURE_SERVER(
+            "esriFeatureServer"), MAPBOX_VECTOR_TILE("mvt"), PMTILES("pmtiles");
 
     private final String typeString;
 
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/RoutingIslandsTest.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/RoutingIslandsTest.java
@@ -179,9 +179,11 @@ private void runGenericTest(String currentTransportMode, Collection<Way> potenti
                 .filter(way -> !incomingWays.contains(way) || !outgoingWays.contains(way))
                 .filter(way -> Access.getPositiveAccessValues().contains(
                         getDefaultAccessTags(way).getOrDefault(currentTransportMode, Access.AccessTags.NO.getKey())))
-                .collect(Collectors.toSet())).stream()
-                .map(way -> new Pair<>((incomingWays.containsAll(way) ? marktr("outgoing") : marktr("incoming")), way))
-                .toList();
+                .collect(Collectors.toSet()))
+                        .stream()
+                        .map(way -> new Pair<>(
+                                (incomingWays.containsAll(way) ? marktr("outgoing") : marktr("incoming")), way))
+                        .toList();
         createErrors(problematic, currentTransportMode);
     }
 
@@ -328,8 +330,7 @@ public static boolean checkAccessibility(Way from, Way to, String currentTranspo
         var isAccessible = true;
 
         List<Relation> relations = from.getReferrers().stream().distinct().filter(Relation.class::isInstance)
-                .map(Relation.class::cast).filter(relation -> "restriction".equals(relation.get("type")))
-                .toList();
+                .map(Relation.class::cast).filter(relation -> "restriction".equals(relation.get("type"))).toList();
         for (Relation relation : relations) {
             if (((relation.hasKey("except") && relation.get("except").contains(currentTransportMode))
                     || (currentTransportMode == null || currentTransportMode.trim().isEmpty()))
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/gui/preferences/mapwithai/MapWithAIProvidersPanel.java
@@ -503,7 +503,7 @@ public void mouseClicked(MouseEvent e) {
      * @param e The MouseEvent (used to get the appropriate JTable)
      */
     private static void clickListener(MouseEvent e) {
-        if (e.getSource() instanceof JTable table && table.getSelectedRow() >= 0 && table.getSelectedColumn() >= 0) {
+        if (e.getSource()instanceof JTable table && table.getSelectedRow() >= 0 && table.getSelectedColumn() >= 0) {
             final int realCol = table.convertColumnIndexToModel(table.getSelectedColumn());
             final int realRow = table.convertRowIndexToModel(table.getSelectedRow());
             final var tableName = table.getModel().getColumnName(realCol);
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReader.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/io/mapwithai/ESRISourceReader.java
@@ -119,10 +119,10 @@ public List<ForkJoinTask<MapWithAIInfo>> parse() throws IOException {
                 /* Do nothing */
                 if (parser.hasNext() && parser.next() == JsonParser.Event.START_OBJECT) {
                     parser.getObjectStream().forEach(entry -> {
-                        if ("nextStart".equals(entry.getKey()) && entry.getValue() instanceof JsonNumber number) {
+                        if ("nextStart".equals(entry.getKey()) && entry.getValue()instanceof JsonNumber number) {
                             next.set(number.intValue());
                             searchUrl.set(startReplace.matcher(search).replaceAll(Integer.toString(next.get())));
-                        } else if ("results".equals(entry.getKey()) && entry.getValue() instanceof JsonArray features) {
+                        } else if ("results".equals(entry.getKey()) && entry.getValue()instanceof JsonArray features) {
                             for (var feature : features.getValuesAs(JsonObject.class)) {
                                 information.add(parse(feature));
                             }
diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/tools/MapPaintUtils.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/tools/MapPaintUtils.java
@@ -60,12 +60,13 @@ public final class MapPaintUtils {
      * Safe colors
      */
     public enum SafeColors {
-        RED(Color.RED), ORANGE(Color.ORANGE), GOLD(Objects.requireNonNull(ColorHelper.html2color("#ffd700"))),
-        LIME(Objects.requireNonNull(ColorHelper.html2color("#00ff00"))), CYAN(Color.CYAN),
-        DODGER_BLUE(Objects.requireNonNull(ColorHelper.html2color("#1e90ff"))),
-        AI_MAGENTA(Objects.requireNonNull(ColorHelper.html2color("#ff26d4"))), PINK(Color.PINK),
-        LIGHT_GREY(Objects.requireNonNull(ColorHelper.html2color("#d3d3d3"))),
-        LINEN(Objects.requireNonNull(ColorHelper.html2color("#faf0e6")));
+        RED(Color.RED), ORANGE(Color.ORANGE), GOLD(Objects.requireNonNull(ColorHelper.html2color("#ffd700"))), LIME(
+                Objects.requireNonNull(ColorHelper.html2color("#00ff00"))), CYAN(Color.CYAN), DODGER_BLUE(
+                        Objects.requireNonNull(ColorHelper.html2color("#1e90ff"))), AI_MAGENTA(
+                                Objects.requireNonNull(ColorHelper.html2color("#ff26d4"))), PINK(
+                                        Color.PINK), LIGHT_GREY(
+                                                Objects.requireNonNull(ColorHelper.html2color("#d3d3d3"))), LINEN(
+                                                        Objects.requireNonNull(ColorHelper.html2color("#faf0e6")));
 
         private final Color color;
 

Original file line number	Diff line number	Diff line change
`@@ -275,7 +275,7 @@ private static void realCleanup(DataSet dataSet, Bounds bounds, MapWithAIInfo in`
`275`	`275`	`}`
`276`	`276`	`(boundsToUse.isCollapsed() \|\| boundsToUse.isOutOfTheWorld() ? dataSet.getWays()`
`277`	`277`	`: dataSet.searchWays(boundsToUse.toBBox())).stream().filter(way -> !way.isDeleted())`
`278`		`- .forEach(GetDataRunnable::cleanupArtifacts);`
	`278`	`+ .forEach(GetDataRunnable::cleanupArtifacts);`
`279`	`279`	`}`
`280`	`280`
`281`	`281`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -283,7 +283,7 @@ public List<MapWithAIInfo> compute() {`
`283`	`283`	`}`
`284`	`284`	`// This is literally to avoid allocations on startup`
`285`	`285`	`final Preferences preferences;`
`286`		`- if (Config.getPref() instanceof Preferences pref) {`
	`286`	`+ if (Config.getPref()instanceof Preferences pref) {`
`287`	`287`	`preferences = pref;`
`288`	`288`	`} else {`
`289`	`289`	`preferences = null;`