From 98031cc0f3a6579a822a5c3b2a240454f589033b Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Wed, 5 Jan 2022 13:14:33 -0700 Subject: [PATCH] Remove Esri detection for too many objects This is due to issues with streams and parsing JSON. Signed-off-by: Taylor Smock --- .../BoundingBoxMapWithAIDownloader.java | 35 ++-------------- .../BoundingBoxMapWithAIDownloaderTest.java | 41 ------------------- 2 files changed, 3 insertions(+), 73 deletions(-) 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 7076857..30f5b15 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 @@ -3,13 +3,7 @@ package org.openstreetmap.josm.plugins.mapwithai.backend; import static org.openstreetmap.josm.tools.I18n.tr; -import javax.json.Json; -import javax.json.JsonValue; -import javax.json.stream.JsonParser; - import java.awt.geom.Area; -import java.io.BufferedInputStream; -import java.io.IOException; import java.io.InputStream; import java.net.SocketTimeoutException; import java.util.Arrays; @@ -42,7 +36,6 @@ import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIType; import org.openstreetmap.josm.plugins.mapwithai.tools.MapPaintUtils; import org.openstreetmap.josm.tools.HttpClient; -import org.openstreetmap.josm.tools.JosmRuntimeException; import org.openstreetmap.josm.tools.Logging; /** @@ -181,31 +174,9 @@ public class BoundingBoxMapWithAIDownloader extends BoundingBoxDownloader { // Fall back to Esri Feature Server check. They don't always indicate a json // return type. :( || this.info.getSourceType() == MapWithAIType.ESRI_FEATURE_SERVER) { - if (!source.markSupported()) { - source = new BufferedInputStream(source); - } - source.mark(1024); - try (JsonParser parser = Json.createParser(source)) { - while (parser.hasNext()) { - JsonParser.Event event = parser.next(); - if (event == JsonParser.Event.START_OBJECT) { - parser.getObjectStream().filter(entry -> "properties".equals(entry.getKey())) - .filter(entry -> entry.getValue().getValueType() == JsonValue.ValueType.OBJECT) - .map(entry -> entry.getValue().asJsonObject()) - .filter(value -> value.containsKey("exceededTransferLimit") && value - .get("exceededTransferLimit").getValueType() == JsonValue.ValueType.TRUE) - .findFirst().ifPresent(val -> { - Logging.error("Could not fully download {0}", this.downloadArea); - }); - } - } - source.reset(); - ds = GeoJSONReader.parseDataSet(source, progressMonitor); - if (info.getReplacementTags() != null) { - GetDataRunnable.replaceKeys(ds, info.getReplacementTags()); - } - } catch (IOException e) { - throw new JosmRuntimeException(e); + ds = GeoJSONReader.parseDataSet(source, progressMonitor); + if (info.getReplacementTags() != null) { + GetDataRunnable.replaceKeys(ds, info.getReplacementTags()); } } else { // Fall back to XML parsing diff --git a/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloaderTest.java b/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloaderTest.java index 1f4871f..d2fec26 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloaderTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/BoundingBoxMapWithAIDownloaderTest.java @@ -4,19 +4,11 @@ package org.openstreetmap.josm.plugins.mapwithai.backend; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; -import javax.json.Json; -import javax.json.JsonObjectBuilder; - -import java.util.ArrayList; -import java.util.List; import java.util.stream.Collectors; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.gui.progress.NullProgressMonitor; @@ -28,7 +20,6 @@ import org.openstreetmap.josm.testutils.JOSMTestRules; import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.openstreetmap.josm.testutils.annotations.BasicWiremock; import org.openstreetmap.josm.testutils.annotations.HTTP; -import org.openstreetmap.josm.tools.Logging; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.admin.model.GetServeEventsResult; @@ -54,38 +45,6 @@ class BoundingBoxMapWithAIDownloaderTest { @BasicWiremock public WireMockServer wireMockServer; - @ParameterizedTest - @ValueSource(strings = { "text/json", "application/json", "application/geo+json" }) - void testEsriExceededTransferLimit(String responseType) { - final Bounds downloadBounds = new Bounds(10, 10, 20, 20); - final MapWithAIInfo info = new MapWithAIInfo("testEsriExceededTransferLimit", - this.wireMockServer.baseUrl() + "/esriExceededLimit"); - final BoundingBoxMapWithAIDownloader boundingBoxMapWithAIDownloader = new BoundingBoxMapWithAIDownloader( - downloadBounds, info, false); - final JsonObjectBuilder objectBuilder = Json.createObjectBuilder(); - objectBuilder.add("type", "FeatureCollection"); - objectBuilder.add("properties", Json.createObjectBuilder().add("exceededTransferLimit", true)); - objectBuilder.add("features", Json.createArrayBuilder()); - wireMockServer.stubFor(WireMock - .get(boundingBoxMapWithAIDownloader - .getRequestForBbox(downloadBounds.getMinLon(), downloadBounds.getMinLat(), - downloadBounds.getMinLon(), downloadBounds.getMinLat()) - .replace(wireMockServer.baseUrl(), "")) - .willReturn(WireMock.aResponse().withHeader("Content-Type", responseType) - .withBody(objectBuilder.build().toString()))); - - Logging.clearLastErrorAndWarnings(); - final DataSet ds = assertDoesNotThrow( - () -> boundingBoxMapWithAIDownloader.parseOsm(NullProgressMonitor.INSTANCE)); - List errors = new ArrayList<>(Logging.getLastErrorAndWarnings()); - // Needed to avoid CI failures - errors.removeIf(str -> str.contains("Failed to persist preferences")); - assertEquals(1, errors.size(), - "We weren't handling transfer limit issues. Are we now?\n" + String.join("\n", errors)); - assertTrue(errors.get(0).contains("Could not fully download")); - assertTrue(ds.isEmpty()); - } - @Test void testThirdPartyConflation() { MapWithAIInfo.THIRD_PARTY_CONFLATE.put(true);