Skip to content

Commit

Permalink
Fix #22683: IllegalArgumentException: Listener was already added
Browse files Browse the repository at this point in the history
Signed-off-by: Taylor Smock <tsmock@meta.com>
  • Loading branch information
tsmock committed Jan 30, 2023
1 parent 0083585 commit e70f64e
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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}
Expand Down Expand Up @@ -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<String, StringValuePattern> 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<LogRecord> 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<LogRecord> 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);
}
}
}

0 comments on commit e70f64e

Please sign in to comment.