From d52ba178f58431a8c1602385ef575246859140a6 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 7 May 2020 10:21:48 -0600 Subject: [PATCH] Try to detect when download area is too large (from server response), and try again Signed-off-by: Taylor Smock --- .../BoundingBoxMapWithAIDownloader.java | 49 ++++++++++++++++ .../mapwithai/backend/GetDataRunnable.java | 14 ++++- .../mapwithai/backend/MapWithAIDataUtils.java | 57 ++++++++++++++++--- .../backend/GetDataRunnableTest.java | 6 +- .../backend/MapWithAIDataUtilsTest.java | 11 ++-- .../mapwithai/backend/MapWithAILayerTest.java | 6 +- 6 files changed, 124 insertions(+), 19 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 1cd6a0b..0d83d8a 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 @@ -4,12 +4,20 @@ package org.openstreetmap.josm.plugins.mapwithai.backend; import static org.openstreetmap.josm.tools.I18n.tr; import java.io.InputStream; +import java.net.SocketTimeoutException; +import java.util.concurrent.TimeUnit; import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.Notification; +import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.io.BoundingBoxDownloader; import org.openstreetmap.josm.io.IllegalDataException; +import org.openstreetmap.josm.io.OsmApiException; +import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin; import org.openstreetmap.josm.plugins.mapwithai.data.mapwithai.MapWithAIInfo; import org.openstreetmap.josm.tools.HttpClient; @@ -18,6 +26,8 @@ public class BoundingBoxMapWithAIDownloader extends BoundingBoxDownloader { private final String url; private final boolean crop; + private static long lastErrorTime; + private final Bounds downloadArea; private MapWithAIInfo info; @@ -37,6 +47,45 @@ public class BoundingBoxMapWithAIDownloader extends BoundingBoxDownloader { + (crop ? "&crop_bbox=" + DetectTaskingManagerUtils.getTaskingManagerBBox().toStringCSV(",") : ""); } + @Override + public DataSet parseOsm(ProgressMonitor progressMonitor) throws OsmTransferException { + long startTime = System.nanoTime(); + try { + return super.parseOsm(progressMonitor); + } catch (OsmApiException e) { + if (!(e.getResponseCode() == 504 && (System.nanoTime() - lastErrorTime) < 120_000_000_000L)) { + throw e; + } + } catch (OsmTransferException e) { + if (e.getCause() instanceof SocketTimeoutException && (System.nanoTime() - startTime) > 30_000_000_000L) { + updateLastErrorTime(System.nanoTime()); + Notification note = new Notification(); + GuiHelper.runInEDT(() -> note.setContent(tr( + "Attempting to download data in the background. This may fail or succeed in a few minutes."))); + GuiHelper.runInEDT(note::show); + } else { + throw e; + } + } + DataSet ds = new DataSet(); + GetDataRunnable runnable = new GetDataRunnable(downloadArea.toBBox(), ds, NullProgressMonitor.INSTANCE); + MainApplication.worker.execute(() -> { + try { + // It seems that the server has issues if I make a request soon + // after the failing request due to a timeout. + TimeUnit.SECONDS.sleep(10); + } catch (InterruptedException e1) { + Thread.currentThread().interrupt(); + } + runnable.compute(); + }); + return ds; + } + + private static void updateLastErrorTime(long time) { + lastErrorTime = time; + } + @Override protected DataSet parseDataSet(InputStream source, ProgressMonitor progressMonitor) throws IllegalDataException { DataSet ds = OsmReaderCustom.parseDataSet(source, progressMonitor, true); 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 index 4e94c8f..c0beb82 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnable.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnable.java @@ -56,6 +56,8 @@ public class GetDataRunnable extends RecursiveTask { private final transient DataSet dataSet; private final transient ProgressMonitor monitor; + private Integer maximumDimensions; + private static final int MAX_NUMBER_OF_BBOXES_TO_PROCESS = 1; private static final String SERVER_ID_KEY = "server_id"; @@ -86,9 +88,19 @@ public class GetDataRunnable extends RecursiveTask { this.monitor = Optional.ofNullable(monitor).orElse(NullProgressMonitor.INSTANCE); } + /** + * Set the maximum download bbox size. Must be called before execution. + * + * @param maximumDimensions The maximum bbox download size + */ + public void setMaximumDimensions(int maximumDimensions) { + this.maximumDimensions = maximumDimensions; + } + @Override public DataSet compute() { - final List bboxes = MapWithAIDataUtils.reduceBBoxSize(bbox); + final List bboxes = maximumDimensions == null ? MapWithAIDataUtils.reduceBBoxSize(bbox) + : MapWithAIDataUtils.reduceBBoxSize(bbox, maximumDimensions); monitor.beginTask(tr("Downloading {0} data ({1} total downloads)", MapWithAIPlugin.NAME, bboxes.size()), bboxes.size() - 1); if (!monitor.isCanceled()) { diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java index d57f101..5b4c942 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtils.java @@ -47,7 +47,7 @@ import org.openstreetmap.josm.tools.Utils; */ public final class MapWithAIDataUtils { /** THe maximum dimensions for MapWithAI data (in kilometers) */ - public static final int MAXIMUM_SIDE_DIMENSIONS = 5_000; // RapiD is about 1km, max is 10km, but 10km causes + public static final int MAXIMUM_SIDE_DIMENSIONS = 10_000; // RapiD is about 1km, max is 10km, but 10km causes // timeouts private static final int TOO_MANY_BBOXES = 4; private static ForkJoinPool forkJoinPool; @@ -148,20 +148,45 @@ public final class MapWithAIDataUtils { * @return A DataSet with data inside the bbox */ public static DataSet getData(BBox bbox) { - return getData(Arrays.asList(bbox)); + return getData(Arrays.asList(bbox), MAXIMUM_SIDE_DIMENSIONS); } /** + * Get a dataset from the API servers using a bbox + * + * @param bbox The bbox from which to get data + * @param maximumDimensions The maximum dimensions to try to download at any one + * time + * @return A DataSet with data inside the bbox + */ + public static DataSet getData(BBox bbox, int maximumDimensions) { + return getData(Arrays.asList(bbox), maximumDimensions); + } + + /** + * * Get a dataset from the API servers using a list bboxes * * @param bbox The bboxes from which to get data * @return A DataSet with data inside the bboxes */ public static DataSet getData(List bbox) { + return getData(bbox, MAXIMUM_SIDE_DIMENSIONS); + } + + /** + * Get a dataset from the API servers using a list bboxes + * + * @param bbox The bboxes from which to get data + * @param maximumDimensions The maximum dimensions to try to download at any one + * time + * @return A DataSet with data inside the bboxes + */ + public static DataSet getData(List bbox, int maximumDimensions) { final DataSet dataSet = new DataSet(); final List realBBoxes = bbox.stream().filter(BBox::isValid).distinct().collect(Collectors.toList()); final List realBounds = realBBoxes.stream() - .flatMap(tBBox -> MapWithAIDataUtils.reduceBBoxSize(tBBox).stream()) + .flatMap(tBBox -> MapWithAIDataUtils.reduceBBoxSize(tBBox, maximumDimensions).stream()) .map(MapWithAIDataUtils::bboxToBounds).collect(Collectors.toList()); if (!MapWithAILayerInfo.instance.getLayers().isEmpty()) { if ((realBBoxes.size() < TOO_MANY_BBOXES) || confirmBigDownload(realBBoxes)) { @@ -179,6 +204,9 @@ public final class MapWithAIDataUtils { } } catch (OsmTransferException e) { Logging.error(e); + if (maximumDimensions > MAXIMUM_SIDE_DIMENSIONS / 10) { + dataSet.mergeFrom(getData(bound.toBBox(), maximumDimensions / 2)); + } } })); monitor.finishTask(); @@ -423,16 +451,17 @@ public final class MapWithAIDataUtils { } /** - * @param bbox The bbox to reduce to a set maximum dimension + * @param bbox The bbox to reduce to a set maximum dimension + * @param maximumDimensions The maximum side dimensions of the bbox * @return A list of BBoxes that have a dimension no more than - * {@link MAXIMUM_SIDE_DIMENSIONS} + * {@code maximumDimensions} */ - public static List reduceBBoxSize(BBox bbox) { + public static List reduceBBoxSize(BBox bbox, int maximumDimensions) { final List returnBounds = new ArrayList<>(); final double width = getWidth(bbox); final double height = getHeight(bbox); - final Double widthDivisions = width / MAXIMUM_SIDE_DIMENSIONS; - final Double heightDivisions = height / MAXIMUM_SIDE_DIMENSIONS; + final Double widthDivisions = width / maximumDimensions; + final Double heightDivisions = height / maximumDimensions; final int widthSplits = widthDivisions.intValue() + ((widthDivisions - widthDivisions.intValue()) > 0 ? 1 : 0); final int heightSplits = heightDivisions.intValue() + ((heightDivisions - heightDivisions.intValue()) > 0 ? 1 : 0); @@ -458,8 +487,18 @@ public final class MapWithAIDataUtils { * {@link MAXIMUM_SIDE_DIMENSIONS} */ public static List reduceBBoxSize(List bboxes) { + return reduceBBoxSize(bboxes, MAXIMUM_SIDE_DIMENSIONS); + } + + /** + * @param bboxes The bboxes to reduce to a set maximum dimension + * @param maximumDimensions The maximum width/height dimensions + * @return A list of BBoxes that have a dimension no more than the + * {@code maximumDimensions} + */ + public static List reduceBBoxSize(List bboxes, int maximumDimensions) { final List returnBBoxes = new ArrayList<>(); - bboxes.forEach(bbox -> returnBBoxes.addAll(reduceBBoxSize(bbox))); + bboxes.forEach(bbox -> returnBBoxes.addAll(reduceBBoxSize(bbox, maximumDimensions))); return returnBBoxes.stream().distinct().collect(Collectors.toList()); } diff --git a/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnableTest.java b/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnableTest.java index dc75a19..18b71a0 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnableTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnableTest.java @@ -98,8 +98,10 @@ public class GetDataRunnableTest { @Test public void testRegressionTicket46() { DataSet ds = new DataSet(); - new GetDataRunnable(Arrays.asList(new BBox(-5.7400005, 34.4524384, -5.6686014, 34.5513153)), ds, null).fork() - .join(); + GetDataRunnable getData = new GetDataRunnable( + Arrays.asList(new BBox(-5.7400005, 34.4524384, -5.6686014, 34.5513153)), ds, null); + getData.setMaximumDimensions(5_000); + getData.fork().join(); assertNotNull(ds); assertFalse(ds.isEmpty()); assertFalse(ds.allNonDeletedPrimitives().isEmpty()); diff --git a/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtilsTest.java b/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtilsTest.java index 34f9648..59085e9 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtilsTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIDataUtilsTest.java @@ -175,8 +175,9 @@ public class MapWithAIDataUtilsTest { final BBox bbox = new BBox(0, 0, 0.0001, 0.0001); for (Double i : Arrays.asList(0.0001, 0.001, 0.01, 0.1)) { bbox.add(i, i); - List bboxes = MapWithAIDataUtils.reduceBBoxSize(bbox); - assertEquals(getExpectedNumberOfBBoxes(bbox), bboxes.size(), "The bbox should be appropriately reduced"); + List bboxes = MapWithAIDataUtils.reduceBBoxSize(bbox, 5_000); + assertEquals(getExpectedNumberOfBBoxes(bbox, 5_000), bboxes.size(), + "The bbox should be appropriately reduced"); checkInBBox(bbox, bboxes); checkBBoxesConnect(bbox, bboxes); } @@ -192,11 +193,11 @@ public class MapWithAIDataUtilsTest { .collect(Collectors.toList()).isEmpty()); } - private static int getExpectedNumberOfBBoxes(BBox bbox) { + private static int getExpectedNumberOfBBoxes(BBox bbox, int maximumDimensions) { double width = MapWithAIDataUtils.getWidth(bbox); double height = MapWithAIDataUtils.getHeight(bbox); - int widthDivisions = (int) Math.ceil(width / MapWithAIDataUtils.MAXIMUM_SIDE_DIMENSIONS); - int heightDivisions = (int) Math.ceil(height / MapWithAIDataUtils.MAXIMUM_SIDE_DIMENSIONS); + int widthDivisions = (int) Math.ceil(width / maximumDimensions); + int heightDivisions = (int) Math.ceil(height / maximumDimensions); return widthDivisions * heightDivisions; } diff --git a/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAILayerTest.java b/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAILayerTest.java index dd07404..d44a382 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAILayerTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAILayerTest.java @@ -142,8 +142,10 @@ public class MapWithAILayerTest { public void testSelection() throws InvocationTargetException, InterruptedException { MapWithAILayer mapWithAILayer = MapWithAIDataUtils.getLayer(true); DataSet ds = mapWithAILayer.getDataSet(); - new GetDataRunnable(Arrays.asList(new BBox(-5.7400005, 34.4524384, -5.6686014, 34.5513153)), ds, null).fork() - .join(); + GetDataRunnable getData = new GetDataRunnable( + Arrays.asList(new BBox(-5.7400005, 34.4524384, -5.6686014, 34.5513153)), ds, null); + getData.setMaximumDimensions(5_000); + getData.fork().join(); assertTrue(ds.getSelected().isEmpty()); SwingUtilities.invokeAndWait(() -> ds.setSelected(ds.allNonDeletedCompletePrimitives())); assertEquals(1, ds.getSelected().size());