Add code to avoid accidentally uploading bad data in older plugin versions.

This and related commits will be backported.

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

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAILayer.java

@@ -26,6 +26,7 @@ import javax.swing.Action;
 import javax.swing.Icon;
 import javax.swing.JCheckBoxMenuItem;
 import javax.swing.JLabel;
+import javax.swing.JOptionPane;
 import javax.swing.JPanel;
 import javax.swing.SwingConstants;
 
@@ -38,6 +39,7 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.UploadPolicy;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.Notification;
 import org.openstreetmap.josm.gui.layer.Layer;
 import org.openstreetmap.josm.gui.layer.MainLayerManager.ActiveLayerChangeEvent;
 import org.openstreetmap.josm.gui.layer.MainLayerManager.ActiveLayerChangeListener;
@@ -45,8 +47,10 @@ import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.mappaint.MapPaintStyles;
 import org.openstreetmap.josm.gui.mappaint.StyleSource;
 import org.openstreetmap.josm.gui.util.GuiHelper;
+import org.openstreetmap.josm.gui.widgets.HtmlPanel;
 import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
 import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo;
+import org.openstreetmap.josm.plugins.mapwithai.tools.BlacklistUtils;
 import org.openstreetmap.josm.plugins.mapwithai.tools.MapPaintUtils;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.GBC;
@@ -227,6 +231,13 @@ public class MapWithAILayer extends OsmDataLayer implements ActiveLayerChangeLis
 
     @Override
     public void selectionChanged(SelectionChangeEvent event) {
+        if (BlacklistUtils.isBlacklisted()) {
+            if (!event.getSelection().isEmpty()) {
+                GuiHelper.runInEDT(() -> getDataSet().setSelected(Collections.emptySet()));
+                createBadDataNotification();
+            }
+            return;
+        }
         super.selectionChanged(event);
         final int maximumAdditionSelection = MapWithAIPreferenceHelper.getMaximumAddition();
         if ((event.getSelection().size() - event.getOldSelection().size() > 1
@@ -248,6 +259,29 @@ public class MapWithAILayer extends OsmDataLayer implements ActiveLayerChangeLis
         }
     }
 
+    /**
+     * Create a notification for plugin versions that create bad data.
+     */
+    public static void createBadDataNotification() {
+        Notification badData = new Notification();
+        badData.setIcon(JOptionPane.ERROR_MESSAGE);
+        badData.setDuration(Notification.TIME_LONG);
+        HtmlPanel panel = new HtmlPanel();
+        StringBuilder message = new StringBuilder()
+                .append(tr("This version of the MapWithAI plugin is known to create bad data.")).append("<br />")
+                .append(tr("Please update plugins and/or JOSM."));
+        if (BlacklistUtils.isOffline()) {
+            message.append("<br />").append(tr("This message may occur when JOSM is offline."));
+        }
+        panel.setText(message.toString());
+        badData.setContent(panel);
+        MapWithAILayer layer = MapWithAIDataUtils.getLayer(false);
+        if (layer != null) {
+            layer.setMaximumAddition(0);
+        }
+        GuiHelper.runInEDT(badData::show);
+    }
+
     private static class OsmComparator implements Comparator<OsmPrimitive>, Serializable {
         final Collection<OsmPrimitive> previousSelection;
 

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveAction.java

@@ -27,6 +27,7 @@ import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
 import org.openstreetmap.josm.plugins.mapwithai.commands.MapWithAIAddCommand;
+import org.openstreetmap.josm.plugins.mapwithai.tools.BlacklistUtils;
 import org.openstreetmap.josm.tools.Shortcut;
 
 public class MapWithAIMoveAction extends JosmAction {
@@ -63,6 +64,10 @@ public class MapWithAIMoveAction extends JosmAction {
 
     @Override
     public void actionPerformed(ActionEvent event) {
+        if (BlacklistUtils.isBlacklisted()) {
+            MapWithAILayer.createBadDataNotification();
+            return;
+        }
         for (final MapWithAILayer mapWithAI : MainApplication.getLayerManager().getLayersOfType(MapWithAILayer.class)) {
             final DataSet ds = mapWithAI.getDataSet();
             final int maxAddition = MapWithAIPreferenceHelper.getMaximumAddition();

src/main/java/org/openstreetmap/josm/plugins/mapwithai/tools/BlacklistUtils.java

@@ -14,6 +14,7 @@ import javax.json.JsonStructure;
 import javax.json.JsonValue;
 
 import org.openstreetmap.josm.io.CachedFile;
+import org.openstreetmap.josm.io.NetworkManager;
 import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
 import org.openstreetmap.josm.tools.Logging;
 
@@ -39,8 +40,8 @@ public class BlacklistUtils {
      */
     public static boolean isBlacklisted() {
         String version = MapWithAIPlugin.getVersionInfo();
-        try (CachedFile blacklist = new CachedFile(blacklistUrl);
+        CachedFile blacklist = new CachedFile(blacklistUrl);
-                BufferedReader bufferedReader = blacklist.getContentReader();
+        try (BufferedReader bufferedReader = blacklist.getContentReader();
                 JsonReader reader = Json.createReader(bufferedReader)) {
             JsonStructure structure = reader.read();
             if (structure.getValueType() == JsonValue.ValueType.ARRAY) {
@@ -52,7 +53,14 @@ public class BlacklistUtils {
                 return object.keySet().contains(version);
             }
         } catch (IOException | JsonException e) {
+            try {
+                blacklist.clear();
+            } catch (IOException e1) {
+                Logging.error(e1);
+            }
             Logging.error(e);
+        } finally {
+            blacklist.close();
         }
         return true;
     }
@@ -65,4 +73,13 @@ public class BlacklistUtils {
     static void setBlacklistUrl(String url) {
         blacklistUrl = url;
     }
+
+    /**
+     * Check if we can reach the URL for the known-bad version list.
+     *
+     * @return {@code true} if the configured url is offline
+     */
+    public static boolean isOffline() {
+        return NetworkManager.isOffline(blacklistUrl);
+    }
 }

test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAILayerTest.java

@@ -22,6 +22,7 @@ import javax.swing.JPanel;
 import javax.swing.SwingUtilities;
 
 import org.awaitility.Durations;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -42,6 +43,7 @@ import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
 import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAILayer.ContinuousDownloadAction;
 import org.openstreetmap.josm.plugins.mapwithai.commands.MapWithAIAddCommand;
 import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo;
+import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAIPluginMock;
 import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAITestRules;
 import org.openstreetmap.josm.plugins.mapwithai.tools.MapPaintUtils;
 import org.openstreetmap.josm.spi.preferences.Config;
@@ -57,17 +59,23 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 class MapWithAILayerTest {
     @RegisterExtension
     @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    JOSMTestRules test = new MapWithAITestRules().sources().wiremock().preferences().main().projection().fakeAPI()
+    static JOSMTestRules test = new MapWithAITestRules().sources().wiremock().preferences().main().projection()
-            .territories();
+            .fakeAPI().territories();
 
     MapWithAILayer layer;
 
-    @BeforeEach
+    @BeforeAll
-    void setUp() {
+    static void beforeAll() {
-        layer = new MapWithAILayer(new DataSet(), "test", null);
+        TestUtils.assumeWorkingJMockit();
+        new MapWithAIPluginMock();
         Territories.initialize(); // Required to avoid an NPE (see JOSM-19132)
     }
 
+    @BeforeEach
+    void beforeEach() {
+        layer = new MapWithAILayer(new DataSet(), "test", null);
+    }
+
     @Test
     void testGetSource() {
         assertNull(layer.getChangesetSourceTag(), "The source tag should be null");

test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveActionTest.java

@@ -13,6 +13,7 @@ import java.util.concurrent.Future;
 
 import org.awaitility.Awaitility;
 import org.awaitility.Durations;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -28,6 +29,7 @@ import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.plugins.mapwithai.commands.ConnectedCommand;
 import org.openstreetmap.josm.plugins.mapwithai.commands.DuplicateCommand;
+import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAIPluginMock;
 import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAITestRules;
 import org.openstreetmap.josm.plugins.mapwithai.testutils.MissingConnectionTagsMocker;
 import org.openstreetmap.josm.spi.preferences.Config;
@@ -49,9 +51,14 @@ class MapWithAIMoveActionTest {
 
     @RegisterExtension
     @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    JOSMTestRules test = new MapWithAITestRules().wiremock().preferences().main().projection().territories()
+    static JOSMTestRules test = new MapWithAITestRules().wiremock().preferences().main().projection().territories()
             .assertionsInEDT();
 
+    @BeforeAll
+    static void beforeAll() {
+        new MapWithAIPluginMock();
+    }
+
     @BeforeEach
     void setUp() {
         moveAction = new MapWithAIMoveAction();

test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIUploadHookTest.java

@@ -14,6 +14,7 @@ import java.util.Map;
 import java.util.TreeMap;
 import java.util.stream.Collectors;
 
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -30,6 +31,7 @@ import org.openstreetmap.josm.plugins.PluginException;
 import org.openstreetmap.josm.plugins.PluginInformation;
 import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo;
 import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAILayerInfo;
+import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAIPluginMock;
 import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAITestRules;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 
@@ -42,7 +44,8 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 class MapWithAIUploadHookTest {
     @RegisterExtension
     @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    JOSMTestRules test = new MapWithAITestRules().sources().wiremock().main().projection().preferences().territories();
+    static JOSMTestRules test = new MapWithAITestRules().sources().wiremock().main().projection().preferences()
+            .territories();
     private PluginInformation info;
     private OsmDataLayer osmLayer;
     private MapWithAILayer aiLayer;
@@ -51,6 +54,11 @@ class MapWithAIUploadHookTest {
     private MapWithAIMoveAction action;
     private MapWithAIUploadHook hook;
 
+    @BeforeAll
+    static void beforeAll() {
+        new MapWithAIPluginMock();
+    }
+
     @BeforeEach
     void setUp() throws PluginException, IOException {
         try (InputStream in = new ByteArrayInputStream("".getBytes(StandardCharsets.UTF_8))) {

test/unit/org/openstreetmap/josm/plugins/mapwithai/testutils/MapWithAIPluginMock.java

@@ -0,0 +1,14 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.plugins.mapwithai.testutils;
+
+import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
+
+import mockit.Mock;
+import mockit.MockUp;
+
+public class MapWithAIPluginMock extends MockUp<MapWithAIPlugin> {
+    @Mock
+    public static String getVersionInfo() {
+        return "1.0";
+    }
+}

test/unit/org/openstreetmap/josm/plugins/mapwithai/tools/BlacklistUtilsTest.java

@@ -15,14 +15,13 @@ import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
+import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
+import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAIPluginMock;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 
 import com.github.tomakehurst.wiremock.WireMockServer;
 
-import mockit.Mock;
-import mockit.MockUp;
-
 /**
  * Tests for {@link BlacklistUtils}
  *
@@ -31,13 +30,6 @@ import mockit.MockUp;
  */
 class BlacklistUtilsTest {
 
-    private static class MapWithAIPluginMock extends MockUp<MapWithAIPlugin> {
-        @Mock
-        public static String getVersionInfo() {
-            return "1.0";
-        }
-    }
-
     @RegisterExtension
     static JOSMTestRules rules = new JOSMTestRules();
 
@@ -45,6 +37,7 @@ class BlacklistUtilsTest {
 
     @BeforeAll
     static void setup() {
+        TestUtils.assumeWorkingJMockit();
         wireMock = new WireMockServer(options().dynamicPort());
         wireMock.start();
         BlacklistUtils.setBlacklistUrl(wireMock.baseUrl() + "/JOSM_MapWithAI/json/blacklisted_versions.json");