From e70f64e53424a574283d14cb023f558569e871d5 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 26 Jan 2023 12:49:02 -0700 Subject: [PATCH] Fix #22683: IllegalArgumentException: Listener was already added Signed-off-by: Taylor Smock --- .../actions/AddMapWithAILayerAction.java | 1 + .../BoundingBoxMapWithAIDownloader.java | 32 ++++++--- .../actions/AddMapWithAILayerActionTest.java | 72 +++++++++++++++++++ 3 files changed, 97 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerAction.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerAction.java index d71562e4..bbe2257b 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerAction.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerAction.java @@ -36,6 +36,7 @@ * Largely copied from {@link AddImageryLayerAction}. */ public class AddMapWithAILayerAction extends JosmAction implements AdaptableAction { + private static final long serialVersionUID = 1403912860658467920L; private final transient MapWithAIInfo info; /** diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java index a4eb05b4..f8a2b363 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloader.java @@ -20,6 +20,7 @@ import java.net.SocketTimeoutException; import java.net.URL; import java.nio.charset.StandardCharsets; +import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.Comparator; @@ -63,6 +64,11 @@ * @author Taylor Smock */ public class BoundingBoxMapWithAIDownloader extends BoundingBoxDownloader { + /** + * The time that the data URL's were last updated. See #22683 for why this is + * necessary. + */ + private static Instant DATA_LAST_UPDATED = Instant.EPOCH; private final String url; private final boolean crop; private final int start; @@ -156,14 +162,24 @@ public DataSet parseOsm(ProgressMonitor progressMonitor) throws OsmTransferExcep "Attempting to download data in the background. This may fail or succeed in a few minutes."))); GuiHelper.runInEDT(note::show); } else if (e.getCause() instanceof IllegalDataException) { - MapWithAILayerInfo.getInstance().loadDefaults(true, MapWithAIDataUtils.getForkJoinPool(), false, - () -> GuiHelper.runInEDT(() -> { - Notification notification = new Notification(tr( - "MapWithAI layers reloaded. Removing and re-adding the MapWithAI layer may be necessary.")); - notification.setIcon(JOptionPane.INFORMATION_MESSAGE); - notification.setDuration(Notification.TIME_LONG); - notification.show(); - })); + final Instant lastUpdated; + final Instant now; + synchronized (BoundingBoxMapWithAIDownloader.class) { + lastUpdated = DATA_LAST_UPDATED; + now = Instant.now(); + DATA_LAST_UPDATED = now; + } + // Only force an update if the last update time is sufficiently old. + if (now.toEpochMilli() - lastUpdated.toEpochMilli() > TimeUnit.MINUTES.toMillis(10)) { + MapWithAILayerInfo.getInstance().loadDefaults(true, MapWithAIDataUtils.getForkJoinPool(), false, + () -> GuiHelper.runInEDT(() -> { + Notification notification = new Notification(tr( + "MapWithAI layers reloaded. Removing and re-adding the MapWithAI layer may be necessary.")); + notification.setIcon(JOptionPane.INFORMATION_MESSAGE); + notification.setDuration(Notification.TIME_LONG); + notification.show(); + })); + } } else { throw e; } diff --git a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerActionTest.java b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerActionTest.java index cac51f16..42f47910 100644 --- a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerActionTest.java +++ b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/actions/AddMapWithAILayerActionTest.java @@ -15,7 +15,14 @@ import java.awt.image.BufferedImage; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutionException; +import java.util.logging.Handler; +import java.util.logging.LogRecord; +import java.util.stream.Collectors; import org.awaitility.Awaitility; import org.awaitility.Durations; @@ -31,16 +38,22 @@ import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAILayer; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAILayerInfo; +import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAITestRules; import org.openstreetmap.josm.plugins.mapwithai.testutils.annotations.MapWithAISources; import org.openstreetmap.josm.plugins.mapwithai.testutils.annotations.NoExceptions; import org.openstreetmap.josm.testutils.JOSMTestRules; import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.openstreetmap.josm.tools.ImageProvider; +import org.openstreetmap.josm.tools.Logging; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import com.github.tomakehurst.wiremock.matching.AnythingPattern; +import com.github.tomakehurst.wiremock.matching.EqualToPattern; +import com.github.tomakehurst.wiremock.matching.StringValuePattern; +import com.github.tomakehurst.wiremock.matching.UrlPathPattern; /** * Test class for {@link AddMapWithAILayerAction} @@ -133,4 +146,63 @@ void testRemoteIcon() throws IOException, ExecutionException, InterruptedExcepti } } + @Test + void testNonRegression22683() throws ExecutionException, InterruptedException { + final MapWithAIInfo info = MapWithAILayerInfo.getInstance().getLayers().stream() + .filter(i -> i.getName().equalsIgnoreCase("MapWithAI")).findAny().orElse(null); + assertNotNull(info); + final OsmDataLayer layer = new OsmDataLayer(new DataSet(), "testNonRegression22683", null); + layer.getDataSet().addDataSource(new DataSource(new Bounds(0, 0, 0.001, 0.001), "Area 1")); + layer.getDataSet().addDataSource(new DataSource(new Bounds(-0.001, -0.001, 0, 0), "Area 2")); + MainApplication.getLayerManager().addLayer(layer); + final WireMockServer server = new WireMockServer(WireMockConfiguration.options().dynamicPort()); + final Map parameterMap = new HashMap<>(); + final AnythingPattern anythingPattern = new AnythingPattern(); + parameterMap.put("geometryType", anythingPattern); + parameterMap.put("geometry", anythingPattern); + parameterMap.put("inSR", new EqualToPattern("4326")); + parameterMap.put("f", new EqualToPattern("geojson")); + parameterMap.put("outfields", new EqualToPattern("*")); + parameterMap.put("result_type", new EqualToPattern("road_building_vector_xml")); + parameterMap.put("resultOffset", anythingPattern); + server.stubFor(WireMock.get(new UrlPathPattern(new EqualToPattern("/query"), false)) + .withQueryParams(parameterMap).willReturn(WireMock.aResponse().withBody("{\"test\":0}"))); + final List logs = new ArrayList<>(); + Handler testHandler = new Handler() { + @Override + public void publish(LogRecord record) { + logs.add(record); + } + + @Override + public void flush() { + // Do nothing + } + + @Override + public void close() { + // Do nothing + } + }; + Logging.getLogger().addHandler(testHandler); + try { + server.start(); + info.setUrl(server.baseUrl()); + info.setSourceType(MapWithAIType.ESRI_FEATURE_SERVER); + final AddMapWithAILayerAction action = new AddMapWithAILayerAction(info); + Logging.clearLastErrorAndWarnings(); + assertDoesNotThrow(() -> action.actionPerformed(null)); + GuiHelper.runInEDTAndWait(() -> { + /* Sync thread */ }); + MainApplication.worker.submit(() -> { + /* Sync thread */ }).get(); + final List ides = logs.stream() + .filter(record -> record.getThrown() instanceof IllegalArgumentException) + .collect(Collectors.toList()); + assertTrue(ides.isEmpty(), ides.stream().map(LogRecord::getMessage).collect(Collectors.joining("\n"))); + } finally { + server.stop(); + Logging.getLogger().removeHandler(testHandler); + } + } }