Index: src/org/openstreetmap/josm/gui/io/AbstractPrimitiveTask.java =================================================================== --- src/org/openstreetmap/josm/gui/io/AbstractPrimitiveTask.java (revision 15745) +++ src/org/openstreetmap/josm/gui/io/AbstractPrimitiveTask.java (working copy) @@ -20,6 +20,7 @@ import org.openstreetmap.josm.gui.progress.ProgressMonitor; import org.openstreetmap.josm.gui.progress.swing.PleaseWaitProgressMonitor; import org.openstreetmap.josm.gui.util.GuiHelper; +import org.openstreetmap.josm.io.MultiFetchOverpassObjectReader; import org.openstreetmap.josm.io.MultiFetchServerObjectReader; import org.openstreetmap.josm.io.OsmServerObjectReader; import org.openstreetmap.josm.io.OsmTransferException; @@ -41,7 +42,6 @@ protected OsmServerObjectReader objectReader; private boolean zoom; - private boolean downloadRelations; private boolean fullRelation; protected AbstractPrimitiveTask(String title, OsmDataLayer layer) { @@ -70,14 +70,13 @@ } /** - * Sets whether . - * @param downloadRelations {@code true} if + * Sets whether all members of the relation should be downloaded completely. + * @param downloadRelations ignored since xxx * @param fullRelation {@code true} if a full download is required, * i.e., a download including the immediate children of a relation. * @return {@code this} */ public final AbstractPrimitiveTask setDownloadRelations(boolean downloadRelations, boolean fullRelation) { - this.downloadRelations = downloadRelations; this.fullRelation = fullRelation; return this; } @@ -103,6 +102,9 @@ multiObjectReader = MultiFetchServerObjectReader.create(); } initMultiFetchReader(multiObjectReader); + if (multiObjectReader instanceof MultiFetchOverpassObjectReader) { + ((MultiFetchOverpassObjectReader) multiObjectReader).setRecurseDownRelations(fullRelation); + } theirDataSet = multiObjectReader.parseOsm(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); missingPrimitives = multiObjectReader.getMissingPrimitives(); synchronized (this) { @@ -110,7 +112,7 @@ } new DataSetMerger(ds, theirDataSet).merge(); - if (downloadRelations) { + if (fullRelation) { loadIncompleteRelationMembers(); } Index: src/org/openstreetmap/josm/gui/io/UpdatePrimitivesTask.java =================================================================== --- src/org/openstreetmap/josm/gui/io/UpdatePrimitivesTask.java (revision 15745) +++ src/org/openstreetmap/josm/gui/io/UpdatePrimitivesTask.java (working copy) @@ -5,11 +5,9 @@ import java.util.Collection; import java.util.Collections; +import java.util.stream.Collectors; -import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.Relation; -import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.layer.OsmDataLayer; import org.openstreetmap.josm.io.MultiFetchServerObjectReader; @@ -32,41 +30,11 @@ public UpdatePrimitivesTask(OsmDataLayer layer, Collection toUpdate) { super(tr("Update objects"), layer); this.toUpdate = toUpdate != null ? toUpdate : Collections.emptyList(); + this.setDownloadRelations(false, false); } - protected void initMultiFetchReaderWithNodes(MultiFetchServerObjectReader reader) { - getProgressMonitor().indeterminateSubTask(tr("Initializing nodes to update ...")); - for (OsmPrimitive primitive : toUpdate) { - if (primitive instanceof Node && !primitive.isNew()) { - reader.append(primitive); - } - } - } - - protected void initMultiFetchReaderWithWays(MultiFetchServerObjectReader reader) { - getProgressMonitor().indeterminateSubTask(tr("Initializing ways to update ...")); - for (OsmPrimitive primitive : toUpdate) { - if (primitive instanceof Way && !primitive.isNew()) { - // this also adds way nodes - reader.append(primitive); - } - } - } - - protected void initMultiFetchReaderWithRelations(MultiFetchServerObjectReader reader) { - getProgressMonitor().indeterminateSubTask(tr("Initializing relations to update ...")); - for (OsmPrimitive primitive : toUpdate) { - if (primitive instanceof Relation && !primitive.isNew()) { - // this also adds relation members - reader.append(primitive); - } - } - } - @Override protected void initMultiFetchReader(MultiFetchServerObjectReader reader) { - initMultiFetchReaderWithNodes(reader); - initMultiFetchReaderWithWays(reader); - initMultiFetchReaderWithRelations(reader); + reader.append(toUpdate.stream().filter(p -> !p.isNew()).collect(Collectors.toList())); } } Index: src/org/openstreetmap/josm/io/MultiFetchOverpassObjectReader.java =================================================================== --- src/org/openstreetmap/josm/io/MultiFetchOverpassObjectReader.java (revision 15745) +++ src/org/openstreetmap/josm/io/MultiFetchOverpassObjectReader.java (working copy) @@ -5,6 +5,7 @@ import java.util.stream.Collectors; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; +import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; /** @@ -12,16 +13,26 @@ * * @since 9241 */ -class MultiFetchOverpassObjectReader extends MultiFetchServerObjectReader { +public class MultiFetchOverpassObjectReader extends MultiFetchServerObjectReader { + private boolean recurseDownRelations = true; @Override protected String buildRequestString(final OsmPrimitiveType type, Set idPackage) { - final String query = idPackage.stream() - .map(x -> type.getAPIName() + '(' + x + ");>;") - .collect(Collectors.joining("", "(", ");out meta;")); + final String query = type.getAPIName() + "(id:" + + idPackage.stream().map(String::valueOf).collect(Collectors.joining(",")) + ");" + + getRecurseOption(type) + "out meta;"; + Logging.debug("{0} {1}", "Overpass query:", query); return "interpreter?data=" + Utils.encodeUrl(query); } + private String getRecurseOption(final OsmPrimitiveType type) { + if (type == OsmPrimitiveType.WAY) + return ">;"; + if (type == OsmPrimitiveType.RELATION && recurseDownRelations) + return ">>;"; // >>; needed for sub relations + return ""; + } + @Override protected String getBaseUrl() { return OverpassDownloadReader.OVERPASS_SERVER.get(); @@ -33,4 +44,13 @@ // accomplished using >; in the query string above return true; } + + /** + * Should generated queries recurse down on relations? + * @param recurseDownRelations true: yes, recurse down to retrieve the complete relation + */ + public void setRecurseDownRelations(boolean recurseDownRelations) { + this.recurseDownRelations = recurseDownRelations; + } + } Index: src/org/openstreetmap/josm/io/MultiFetchServerObjectReader.java =================================================================== --- src/org/openstreetmap/josm/io/MultiFetchServerObjectReader.java (revision 15745) +++ src/org/openstreetmap/josm/io/MultiFetchServerObjectReader.java (working copy) @@ -382,13 +382,18 @@ progressMonitor.beginTask(trn("Downloading {0} object from ''{1}''", "Downloading {0} objects from ''{1}''", n, n, OsmApi.getOsmApi().getBaseUrl())); try { + boolean ignoreAlreadyDownloaded = outputDataSet != null && outputDataSet.isEmpty(); missingPrimitives = new HashSet<>(); if (isCanceled()) return null; + fetchPrimitives(relations, OsmPrimitiveType.RELATION, progressMonitor); + if (isCanceled()) return null; fetchPrimitives(ways, OsmPrimitiveType.WAY, progressMonitor); + if (outputDataSet != null && ignoreAlreadyDownloaded) { + // avoid to download data that was already downloaded + nodes.removeAll(outputDataSet.getNodes().stream().filter(node -> !node.isIncomplete()).map(OsmPrimitive::getUniqueId).collect(Collectors.toList())); + } if (isCanceled()) return null; fetchPrimitives(nodes, OsmPrimitiveType.NODE, progressMonitor); - if (isCanceled()) return null; - fetchPrimitives(relations, OsmPrimitiveType.RELATION, progressMonitor); if (outputDataSet != null) { outputDataSet.deleteInvisible(); } Index: test/functional/org/openstreetmap/josm/io/MultiFetchServerObjectReaderTest.java =================================================================== --- test/functional/org/openstreetmap/josm/io/MultiFetchServerObjectReaderTest.java (revision 15745) +++ test/functional/org/openstreetmap/josm/io/MultiFetchServerObjectReaderTest.java (working copy) @@ -42,9 +42,9 @@ import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.progress.NullProgressMonitor; import org.openstreetmap.josm.spi.preferences.Config; +import org.openstreetmap.josm.testutils.JOSMTestRules; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import org.openstreetmap.josm.testutils.JOSMTestRules; /** * Unit tests of {@link MultiFetchServerObjectReader}. Index: test/unit/org/openstreetmap/josm/io/MultiFetchOverpassObjectReaderTest.java =================================================================== --- test/unit/org/openstreetmap/josm/io/MultiFetchOverpassObjectReaderTest.java (revision 15745) +++ test/unit/org/openstreetmap/josm/io/MultiFetchOverpassObjectReaderTest.java (working copy) @@ -6,7 +6,6 @@ import java.util.Arrays; import java.util.TreeSet; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Rule; import org.junit.Test; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; @@ -13,6 +12,8 @@ import org.openstreetmap.josm.testutils.JOSMTestRules; import org.openstreetmap.josm.tools.Utils; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * Unit tests of {@link MultiFetchOverpassObjectReader}. */ @@ -29,10 +30,26 @@ * Test {@link MultiFetchOverpassObjectReader#buildRequestString} */ @Test - public void testBuildRequestString() { + public void testBuildRequestWaysString() { String requestString = new MultiFetchOverpassObjectReader() .buildRequestString(OsmPrimitiveType.WAY, new TreeSet<>(Arrays.asList(130L, 123L, 126L))); - assertEquals("interpreter?data=" + Utils.encodeUrl("(way(123);>;way(126);>;way(130);>;);out meta;"), requestString); + assertEquals("interpreter?data=" + Utils.encodeUrl("way(id:123,126,130);>;out meta;"), requestString); } + /** + * Test {@link MultiFetchOverpassObjectReader#buildRequestString} + */ + @Test + public void testBuildRequestRelationsString() { + MultiFetchOverpassObjectReader reader = new MultiFetchOverpassObjectReader(); + reader.setRecurseDownRelations(true); + String requestString = reader.buildRequestString(OsmPrimitiveType.RELATION, + new TreeSet<>(Arrays.asList(130L, 123L, 126L))); + assertEquals("interpreter?data=" + Utils.encodeUrl("relation(id:123,126,130);>>;out meta;"), requestString); + reader.setRecurseDownRelations(false); + requestString = reader.buildRequestString(OsmPrimitiveType.RELATION, + new TreeSet<>(Arrays.asList(130L, 123L, 126L))); + assertEquals("interpreter?data=" + Utils.encodeUrl("relation(id:123,126,130);out meta;"), requestString); + } + }