Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/ChildRelationBrowser.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/ChildRelationBrowser.java (revision 15810) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/ChildRelationBrowser.java (revision 15811) @@ -13,11 +13,8 @@ import java.awt.event.MouseEvent; import java.io.IOException; -import java.net.HttpURLConnection; import java.util.Arrays; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Set; -import java.util.Stack; import java.util.stream.Collectors; @@ -38,5 +35,4 @@ import org.openstreetmap.josm.data.osm.DefaultNameFormatter; import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationMember; @@ -48,8 +44,8 @@ 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.gui.widgets.PopupMenuLauncher; +import org.openstreetmap.josm.io.MultiFetchServerObjectReader; import org.openstreetmap.josm.io.OsmApi; -import org.openstreetmap.josm.io.OsmApiException; -import org.openstreetmap.josm.io.OsmServerObjectReader; import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.tools.CheckParameterUtil; @@ -340,11 +336,33 @@ } + protected MultiFetchServerObjectReader createReader() { + return MultiFetchServerObjectReader.create().setRecurseDownAppended(false).setRecurseDownRelations(true); + } + + /** + * Merges the primitives in ds to the dataset of the edit layer + * + * @param ds the data set + */ + protected void mergeDataSet(DataSet ds) { + if (ds != null) { + final DataSetMerger visitor = new DataSetMerger(getLayer().getDataSet(), ds); + visitor.merge(); + if (!visitor.getConflicts().isEmpty()) { + getLayer().getConflicts().add(visitor.getConflicts()); + conflictsCount += visitor.getConflicts().size(); + } + } + } + protected void refreshView(Relation relation) { - for (int i = 0; i < childTree.getRowCount(); i++) { - Relation reference = (Relation) childTree.getPathForRow(i).getLastPathComponent(); - if (reference == relation) { - model.refreshNode(childTree.getPathForRow(i)); - } - } + GuiHelper.runInEDT(() -> { + for (int i = 0; i < childTree.getRowCount(); i++) { + Relation reference = (Relation) childTree.getPathForRow(i).getLastPathComponent(); + if (reference == relation) { + model.refreshNode(childTree.getPathForRow(i)); + } + } + }); } @@ -375,12 +393,9 @@ */ class DownloadAllChildrenTask extends DownloadTask { - private final Stack relationsToDownload; - private final Set downloadedRelationIds; + private final Relation relation; DownloadAllChildrenTask(Dialog parent, Relation r) { super(tr("Download relation members"), parent); - relationsToDownload = new Stack<>(); - downloadedRelationIds = new HashSet<>(); - relationsToDownload.push(r); + relation = r; } @@ -406,62 +421,14 @@ } - /** - * Remembers the child relations to download - * - * @param parent the parent relation - */ - protected void rememberChildRelationsToDownload(Relation parent) { - downloadedRelationIds.add(parent.getId()); - for (RelationMember member: parent.getMembers()) { - if (member.isRelation()) { - Relation child = member.getRelation(); - if (!downloadedRelationIds.contains(child.getId())) { - relationsToDownload.push(child); - } - } - } - } - - /** - * Merges the primitives in ds to the dataset of the edit layer - * - * @param ds the data set - */ - protected void mergeDataSet(DataSet ds) { - if (ds != null) { - final DataSetMerger visitor = new DataSetMerger(getLayer().getDataSet(), ds); - visitor.merge(); - if (!visitor.getConflicts().isEmpty()) { - getLayer().getConflicts().add(visitor.getConflicts()); - conflictsCount += visitor.getConflicts().size(); - } - } - } - @Override protected void realRun() throws SAXException, IOException, OsmTransferException { try { - while (!relationsToDownload.isEmpty() && !canceled) { - Relation r = relationsToDownload.pop(); - if (r.isNew()) { - continue; - } - rememberChildRelationsToDownload(r); - progressMonitor.setCustomText(tr("Downloading relation {0}", r.getDisplayName(DefaultNameFormatter.getInstance()))); - OsmServerObjectReader reader = new OsmServerObjectReader(r.getId(), OsmPrimitiveType.RELATION, - true); - DataSet dataSet = null; - try { - dataSet = reader.parseOsm(progressMonitor - .createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); - } catch (OsmApiException e) { - if (e.getResponseCode() == HttpURLConnection.HTTP_GONE) { - warnBecauseOfDeletedRelation(r); - continue; - } - throw e; - } - mergeDataSet(dataSet); - refreshView(r); + MultiFetchServerObjectReader reader = createReader(); + reader.append(relation.getMemberPrimitives()); + DataSet dataSet = reader.parseOsm(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); + mergeDataSet(dataSet); + Utils.filteredCollection(reader.getMissingPrimitives(), Relation.class).forEach(this::warnBecauseOfDeletedRelation); + for (Relation rel : dataSet.getRelations()) { + refreshView((Relation) getLayer().getDataSet().getPrimitiveById(rel)); } SwingUtilities.invokeLater(MainApplication.getMap()::repaint); @@ -487,32 +454,16 @@ } - protected void mergeDataSet(DataSet dataSet) { - if (dataSet != null) { - final DataSetMerger visitor = new DataSetMerger(getLayer().getDataSet(), dataSet); - visitor.merge(); - if (!visitor.getConflicts().isEmpty()) { - getLayer().getConflicts().add(visitor.getConflicts()); - conflictsCount += visitor.getConflicts().size(); - } - } - } - @Override protected void realRun() throws SAXException, IOException, OsmTransferException { try { - Iterator it = relations.iterator(); - while (it.hasNext() && !canceled) { - Relation r = it.next(); - if (r.isNew()) { - continue; - } - progressMonitor.setCustomText(tr("Downloading relation {0}", r.getDisplayName(DefaultNameFormatter.getInstance()))); - OsmServerObjectReader reader = new OsmServerObjectReader(r.getId(), OsmPrimitiveType.RELATION, - true); - DataSet dataSet = reader.parseOsm(progressMonitor - .createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); - mergeDataSet(dataSet); - refreshView(r); - } + MultiFetchServerObjectReader reader = createReader(); + reader.append(relations); + DataSet dataSet = reader.parseOsm(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); + mergeDataSet(dataSet); + + for (Relation rel : dataSet.getRelations()) { + refreshView((Relation) getLayer().getDataSet().getPrimitiveById(rel)); + } + } catch (OsmTransferException e) { if (canceled) { Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/DownloadRelationMemberTask.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/DownloadRelationMemberTask.java (revision 15810) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/DownloadRelationMemberTask.java (revision 15811) @@ -130,4 +130,5 @@ objectReader = MultiFetchServerObjectReader.create(); } + objectReader.setRecurseDownAppended(false).setRecurseDownRelations(true); objectReader.append(children); progressMonitor.indeterminateSubTask( Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/DownloadRelationTask.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/DownloadRelationTask.java (revision 15810) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/relation/DownloadRelationTask.java (revision 15811) @@ -12,5 +12,4 @@ import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.DataSetMerger; -import org.openstreetmap.josm.data.osm.DefaultNameFormatter; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.gui.ExceptionDialogUtil; @@ -18,5 +17,7 @@ import org.openstreetmap.josm.gui.PleaseWaitRunnable; import org.openstreetmap.josm.gui.layer.OsmDataLayer; -import org.openstreetmap.josm.io.OsmServerObjectReader; +import org.openstreetmap.josm.gui.progress.ProgressMonitor; +import org.openstreetmap.josm.io.MultiFetchServerObjectReader; +import org.openstreetmap.josm.io.OsmServerReader; import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.tools.CheckParameterUtil; @@ -34,5 +35,5 @@ private final Collection relations; private final OsmDataLayer layer; - private OsmServerObjectReader objectReader; + private OsmServerReader objectReader; /** @@ -78,28 +79,21 @@ try { final DataSet allDownloads = new DataSet(); - int i = 0; getProgressMonitor().setTicksCount(relations.size()); - for (Relation relation: relations) { - i++; - getProgressMonitor().setCustomText(tr("({0}/{1}): Downloading relation ''{2}''...", i, relations.size(), - relation.getDisplayName(DefaultNameFormatter.getInstance()))); - synchronized (this) { - if (canceled) return; - objectReader = new OsmServerObjectReader(relation.getPrimitiveId(), true /* full download */); - } - DataSet dataSet = objectReader.parseOsm( - getProgressMonitor().createSubTaskMonitor(0, false) - ); - if (dataSet == null) + MultiFetchServerObjectReader multiObjectReader; + synchronized (this) { + if (canceled) return; - synchronized (this) { - if (canceled) return; - objectReader = null; - } - DataSetMerger merger = new DataSetMerger(allDownloads, dataSet); - merger.merge(); - getProgressMonitor().worked(1); + multiObjectReader = MultiFetchServerObjectReader.create(); } - + multiObjectReader.setRecurseDownRelations(true).setRecurseDownAppended(false); + multiObjectReader.append(relations); + DataSet dataSet = multiObjectReader.parseOsm(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); + if (dataSet == null) + return; + synchronized (this) { + if (canceled) + return; + } + new DataSetMerger(allDownloads, dataSet).merge(); SwingUtilities.invokeAndWait(() -> { layer.mergeFrom(allDownloads); Index: /trunk/src/org/openstreetmap/josm/gui/io/AbstractPrimitiveTask.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/io/AbstractPrimitiveTask.java (revision 15810) +++ /trunk/src/org/openstreetmap/josm/gui/io/AbstractPrimitiveTask.java (revision 15811) @@ -12,5 +12,4 @@ import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.PrimitiveId; -import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.ExceptionDialogUtil; @@ -42,6 +41,5 @@ private boolean zoom; - private boolean downloadRelations; - private boolean fullRelation; + protected boolean fullRelation; protected AbstractPrimitiveTask(String title, OsmDataLayer layer) { @@ -71,12 +69,11 @@ /** - * Sets whether . - * @param downloadRelations {@code true} if + * Sets whether all members of the relation should be downloaded completely. * @param fullRelation {@code true} if a full download is required, * i.e., a download including the immediate children of a relation. * @return {@code this} + * since 15811 (changed parameter list) */ - public final AbstractPrimitiveTask setDownloadRelations(boolean downloadRelations, boolean fullRelation) { - this.downloadRelations = downloadRelations; + public final AbstractPrimitiveTask setDownloadRelations(boolean fullRelation) { this.fullRelation = fullRelation; return this; @@ -101,5 +98,5 @@ if (canceled) return; - multiObjectReader = MultiFetchServerObjectReader.create(); + multiObjectReader = MultiFetchServerObjectReader.create().setRecurseDownRelations(fullRelation); } initMultiFetchReader(multiObjectReader); @@ -111,8 +108,4 @@ new DataSetMerger(ds, theirDataSet).merge(); - if (downloadRelations) { - loadIncompleteRelationMembers(); - } - loadIncompleteNodes(); } catch (OsmTransferException e) { @@ -120,26 +113,4 @@ return; lastException = e; - } - } - - protected void loadIncompleteRelationMembers() throws OsmTransferException { - // if incomplete relation members exist, download them too - for (Relation r : ds.getRelations()) { - if (canceled) - return; - // Relations may be incomplete in case of nested relations if child relations are accessed before their parent - // (it may happen because "relations" has no deterministic sort order, see #10388) - if (r.isIncomplete() || r.hasIncompleteMembers()) { - synchronized (this) { - if (canceled) - return; - objectReader = new OsmServerObjectReader(r.getId(), OsmPrimitiveType.RELATION, fullRelation); - } - DataSet theirDataSet = objectReader.parseOsm(progressMonitor.createSubTaskMonitor(ProgressMonitor.ALL_TICKS, false)); - synchronized (this) { - objectReader = null; - } - new DataSetMerger(ds, theirDataSet).merge(); - } } } Index: /trunk/src/org/openstreetmap/josm/gui/io/DownloadPrimitivesTask.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/io/DownloadPrimitivesTask.java (revision 15810) +++ /trunk/src/org/openstreetmap/josm/gui/io/DownloadPrimitivesTask.java (revision 15811) @@ -6,9 +6,5 @@ import java.util.List; -import org.openstreetmap.josm.data.osm.Node; -import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.PrimitiveId; -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.gui.progress.ProgressMonitor; @@ -53,5 +49,5 @@ this.ids = ids; setZoom(true); - setDownloadRelations(true, fullRelation); + setDownloadRelations(fullRelation); } @@ -59,22 +55,6 @@ protected void initMultiFetchReader(MultiFetchServerObjectReader reader) { getProgressMonitor().indeterminateSubTask(tr("Initializing nodes to download ...")); - for (PrimitiveId id : ids) { - OsmPrimitive osm = layer.data.getPrimitiveById(id); - if (osm == null) { - switch (id.getType()) { - case NODE: - osm = new Node(id.getUniqueId()); - break; - case WAY: - osm = new Way(id.getUniqueId()); - break; - case RELATION: - osm = new Relation(id.getUniqueId()); - break; - default: throw new AssertionError(); - } - } - reader.append(osm); - } + reader.setRecurseDownRelations(fullRelation); + reader.appendIds(ids); } } Index: /trunk/src/org/openstreetmap/josm/gui/io/UpdatePrimitivesTask.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/io/UpdatePrimitivesTask.java (revision 15810) +++ /trunk/src/org/openstreetmap/josm/gui/io/UpdatePrimitivesTask.java (revision 15811) @@ -7,10 +7,8 @@ import java.util.Collections; -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; +import org.openstreetmap.josm.spi.preferences.Config; /** @@ -33,40 +31,11 @@ super(tr("Update objects"), layer); this.toUpdate = toUpdate != null ? toUpdate : Collections.emptyList(); - } - - 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); - } - } + setDownloadRelations(Config.getPref().getBoolean("update.selected.complete-relation", true)); } @Override protected void initMultiFetchReader(MultiFetchServerObjectReader reader) { - initMultiFetchReaderWithNodes(reader); - initMultiFetchReaderWithWays(reader); - initMultiFetchReaderWithRelations(reader); + // don't update new primitives + toUpdate.stream().filter(p -> !p.isNew()).forEach(reader::append); } } Index: /trunk/src/org/openstreetmap/josm/io/MultiFetchOverpassObjectReader.java =================================================================== --- /trunk/src/org/openstreetmap/josm/io/MultiFetchOverpassObjectReader.java (revision 15810) +++ /trunk/src/org/openstreetmap/josm/io/MultiFetchOverpassObjectReader.java (revision 15811) @@ -2,9 +2,10 @@ package org.openstreetmap.josm.io; +import java.util.Map.Entry; import java.util.Set; import java.util.stream.Collectors; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; -import org.openstreetmap.josm.tools.Utils; +import org.openstreetmap.josm.tools.Logging; /** @@ -13,12 +14,45 @@ * @since 9241 */ -class MultiFetchOverpassObjectReader extends MultiFetchServerObjectReader { +public class MultiFetchOverpassObjectReader extends MultiFetchServerObjectReader { - @Override - protected String buildRequestString(final OsmPrimitiveType type, Set idPackage) { - final String query = idPackage.stream() - .map(x -> type.getAPIName() + '(' + x + ");>;") - .collect(Collectors.joining("", "(", ");out meta;")); - return "interpreter?data=" + Utils.encodeUrl(query); + private static String getPackageString(final OsmPrimitiveType type, Set idPackage) { + return idPackage.stream().map(String::valueOf) + .collect(Collectors.joining(",", type.getAPIName() + (idPackage.size() == 1 ? "(" : "(id:"), ");")); + } + + /** + * Create a single query for all elements + * @return the request string + */ + protected String buildComplexRequestString() { + StringBuilder sb = new StringBuilder(); + int countTypes = 0; + for (Entry> e : primitivesMap.entrySet()) { + if (!e.getValue().isEmpty()) { + countTypes++; + String list = getPackageString(e.getKey(), e.getValue()); + switch (e.getKey()) { + case MULTIPOLYGON: + case RELATION: + sb.append(list); + if (recurseDownRelations) + sb.append(">>;"); + break; + case CLOSEDWAY: + case WAY: + sb.append('(').append(list).append(">;);"); + break; + case NODE: + sb.append(list); + } + } + } + String query = sb.toString(); + if (countTypes > 1) { + query = "(" + query + ");"; + } + query += "out meta;"; + Logging.debug("{0} {1}", "Generated Overpass query:", query); + return query; } @@ -27,10 +61,3 @@ return OverpassDownloadReader.OVERPASS_SERVER.get(); } - - @Override - protected boolean recursesDown() { - // see https://wiki.openstreetmap.org/wiki/Overpass_API/Overpass_QL#Recurse_down_.28.3E.29 for documentation - // accomplished using >; in the query string above - return true; - } } Index: /trunk/src/org/openstreetmap/josm/io/MultiFetchServerObjectReader.java =================================================================== --- /trunk/src/org/openstreetmap/josm/io/MultiFetchServerObjectReader.java (revision 15810) +++ /trunk/src/org/openstreetmap/josm/io/MultiFetchServerObjectReader.java (revision 15811) @@ -13,6 +13,9 @@ import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.Callable; @@ -25,4 +28,5 @@ import java.util.stream.Collectors; +import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.DataSetMerger; @@ -64,5 +68,5 @@ * this leads to a max. request URL of ~ 1900 Bytes ((10 digits + 1 Separator) * 170), * which should be safe according to the - * WWW FAQ. + * WWW FAQ. */ private static final int MAX_IDS_PER_REQUEST = 170; @@ -71,6 +75,10 @@ private final Set ways; private final Set relations; - private Set missingPrimitives; + private final Set missingPrimitives; private final DataSet outputDataSet; + protected final Map> primitivesMap; + + protected boolean recurseDownRelations; + private boolean recurseDownAppended = true; /** @@ -83,4 +91,8 @@ this.outputDataSet = new DataSet(); this.missingPrimitives = new LinkedHashSet<>(); + primitivesMap = new LinkedHashMap<>(); + primitivesMap.put(OsmPrimitiveType.RELATION, relations); + primitivesMap.put(OsmPrimitiveType.WAY, ways); + primitivesMap.put(OsmPrimitiveType.NODE, nodes); } @@ -160,5 +172,5 @@ */ public MultiFetchServerObjectReader appendNode(Node node) { - if (node == null) return this; + if (node == null || node.isNew()) return this; remember(node.getPrimitiveId()); return this; @@ -172,9 +184,10 @@ */ public MultiFetchServerObjectReader appendWay(Way way) { - if (way == null) return this; - if (way.isNew()) return this; - for (Node node: !recursesDown() ? way.getNodes() : Collections.emptyList()) { - if (!node.isNew()) { - remember(node.getPrimitiveId()); + if (way == null || way.isNew()) return this; + if (recurseDownAppended) { + for (Node node : way.getNodes()) { + if (!node.isNew()) { + remember(node.getPrimitiveId()); + } } } @@ -190,15 +203,16 @@ */ protected MultiFetchServerObjectReader appendRelation(Relation relation) { - if (relation == null) return this; - if (relation.isNew()) return this; + if (relation == null || relation.isNew()) return this; remember(relation.getPrimitiveId()); - for (RelationMember member : !recursesDown() ? relation.getMembers() : Collections.emptyList()) { - // avoid infinite recursion in case of cyclic dependencies in relations - if (OsmPrimitiveType.from(member.getMember()) == OsmPrimitiveType.RELATION - && relations.contains(member.getMember().getId())) { - continue; - } - if (!member.getMember().isIncomplete()) { - append(member.getMember()); + if (recurseDownAppended) { + for (RelationMember member : relation.getMembers()) { + // avoid infinite recursion in case of cyclic dependencies in relations + if (OsmPrimitiveType.from(member.getMember()) == OsmPrimitiveType.RELATION + && relations.contains(member.getMember().getId())) { + continue; + } + if (!member.getMember().isIncomplete()) { + append(member.getMember()); + } } } @@ -234,4 +248,36 @@ for (OsmPrimitive primitive : primitives) { append(primitive); + } + return this; + } + + /** + * appends a list of {@link PrimitiveId} to the list of ids which will be fetched from the server. + * + * @param ids the list of primitive Ids (ignored, if null) + * @return this + * @since 15811 + * + */ + public MultiFetchServerObjectReader appendIds(Collection ids) { + if (ids == null) + return this; + for (PrimitiveId id : ids) { + if (id.isNew()) continue; + switch (id.getType()) { + case NODE: + nodes.add(id.getUniqueId()); + break; + case WAY: + case CLOSEDWAY: + ways.add(id.getUniqueId()); + break; + case MULTIPOLYGON: + case RELATION: + relations.add(id.getUniqueId()); + break; + default: + throw new AssertionError(); + } } return this; @@ -276,9 +322,7 @@ protected void rememberNodesOfIncompleteWaysToLoad(DataSet from) { for (Way w: from.getWays()) { - if (w.hasIncompleteNodes()) { - for (Node n: w.getNodes()) { - if (n.isIncomplete()) { - nodes.add(n.getId()); - } + for (Node n: w.getNodes()) { + if (n.isIncomplete()) { + nodes.add(n.getId()); } } @@ -373,4 +417,5 @@ * * @return the parsed data + * @param progressMonitor progress monitor * @throws OsmTransferException if an error occurs while communicating with the API server * @see #getMissingPrimitives() @@ -379,18 +424,28 @@ @Override public DataSet parseOsm(ProgressMonitor progressMonitor) throws OsmTransferException { + missingPrimitives.clear(); int n = nodes.size() + ways.size() + relations.size(); progressMonitor.beginTask(trn("Downloading {0} object from ''{1}''", - "Downloading {0} objects from ''{1}''", n, n, OsmApi.getOsmApi().getBaseUrl())); + "Downloading {0} objects from ''{1}''", n, n, getBaseUrl())); try { - missingPrimitives = new HashSet<>(); - if (isCanceled()) return null; - fetchPrimitives(ways, OsmPrimitiveType.WAY, progressMonitor); - if (isCanceled()) return null; - fetchPrimitives(nodes, OsmPrimitiveType.NODE, progressMonitor); - if (isCanceled()) return null; - fetchPrimitives(relations, OsmPrimitiveType.RELATION, progressMonitor); - if (outputDataSet != null) { - outputDataSet.deleteInvisible(); - } + if (this instanceof MultiFetchOverpassObjectReader) { + // calculate a single request for all the objects + String request = ((MultiFetchOverpassObjectReader) this).buildComplexRequestString(); + if (isCanceled()) + return null; + OverpassDownloadReader reader = new OverpassDownloadReader(new Bounds(0, 0, 0, 0), getBaseUrl(), request); + DataSet ds = reader.parseOsm(progressMonitor.createSubTaskMonitor(n, false)); + new DataSetMerger(outputDataSet, ds).merge(); + checkMissing(outputDataSet, progressMonitor); + } else { + downloadRelations(progressMonitor); + if (isCanceled()) + return null; + fetchPrimitives(ways, OsmPrimitiveType.WAY, progressMonitor); + if (isCanceled()) + return null; + fetchPrimitives(nodes, OsmPrimitiveType.NODE, progressMonitor); + } + outputDataSet.deleteInvisible(); return outputDataSet; } finally { @@ -400,4 +455,69 @@ /** + * Workaround for difference in Oerpass API. + * As of now (version 7.55) Overpass api doesn't return invisible objects. + * Check if we have objects which do not appear in the dataset and fetch them from OSM instead. + * @param ds the dataset + * @param progressMonitor progress monitor + * @throws OsmTransferException if an error occurs while communicating with the API server + */ + private void checkMissing(DataSet ds, ProgressMonitor progressMonitor) throws OsmTransferException { + Set missing = new LinkedHashSet<>(); + for (Entry> e : primitivesMap.entrySet()) { + for (long id : e.getValue()) { + if (ds.getPrimitiveById(id, e.getKey()) == null) + missing.add(e.getKey().newInstance(id, true)); + } + } + if (isCanceled() || missing.isEmpty()) + return; + + MultiFetchServerObjectReader missingReader = MultiFetchServerObjectReader.create(false); + missingReader.setRecurseDownAppended(false); + missingReader.setRecurseDownRelations(false); + missingReader.append(missing); + DataSet mds = missingReader.parseOsm(progressMonitor.createSubTaskMonitor(missing.size(), false)); + new DataSetMerger(ds, mds).merge(); + missingPrimitives.addAll(missingReader.getMissingPrimitives()); + } + + /** + * Finds best way to download a set of relations. + * @param progressMonitor progress monitor + * @throws OsmTransferException if an error occurs while communicating with the API server + * @see #getMissingPrimitives() + */ + private void downloadRelations(ProgressMonitor progressMonitor) throws OsmTransferException { + Set toDownload = new LinkedHashSet<>(relations); + fetchPrimitives(toDownload, OsmPrimitiveType.RELATION, progressMonitor); + if (!recurseDownRelations) { + return; + } + // OSM multi-fetch api may return invisible objects + for (Relation r : outputDataSet.getRelations()) { + if (!r.isVisible()) + toDownload.remove(r.getUniqueId()); + } + while (!toDownload.isEmpty()) { + if (isCanceled()) + return; + final Set addedRelations = new LinkedHashSet<>(); + + for (long id : toDownload) { + OsmServerObjectReader reader = new OsmServerObjectReader(id, OsmPrimitiveType.RELATION, true /* full*/); + DataSet ds = reader.parseOsm(progressMonitor.createSubTaskMonitor(1, false)); + if (ds != null) { + ds.getRelations().stream().map(OsmPrimitive::getUniqueId).filter(uid -> uid != id) + .forEach(addedRelations::add); + } + merge(ds); + } + if (addedRelations.isEmpty()) + break; + toDownload = addedRelations; + } + } + + /** * replies the set of ids of all primitives for which a fetch request to the * server was submitted but which are not available from the server (the server @@ -411,10 +531,23 @@ /** - * Whether this reader fetches nodes when loading ways, or members when loading relations. - * - * @return {@code true} if the reader recurses down - */ - protected boolean recursesDown() { - return false; + * Should downloaded relations be complete? + * @param recurseDownRelations true: yes, recurse down to retrieve the complete relation + * @return this + * @since 15811 + */ + public MultiFetchServerObjectReader setRecurseDownRelations(boolean recurseDownRelations) { + this.recurseDownRelations = recurseDownRelations; + return this; + } + + /** + * Determine how appended objects are treated. By default, all children of an appended object are also appended. + * @param recurseAppended false: do not append known children of appended objects, i.e. all nodes of way and all members of a relation + * @return this + * @since 15811 + */ + public MultiFetchServerObjectReader setRecurseDownAppended(boolean recurseAppended) { + this.recurseDownAppended = recurseAppended; + return this; } @@ -539,4 +672,5 @@ } catch (IOException ex) { Logging.warn(ex); + throw new OsmTransferException(ex); } return result; Index: /trunk/test/unit/org/openstreetmap/josm/io/MultiFetchOverpassObjectReaderTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/io/MultiFetchOverpassObjectReaderTest.java (revision 15810) +++ /trunk/test/unit/org/openstreetmap/josm/io/MultiFetchOverpassObjectReaderTest.java (revision 15811) @@ -5,12 +5,13 @@ import java.util.Arrays; -import java.util.TreeSet; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.testutils.JOSMTestRules; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import org.junit.Rule; -import org.junit.Test; -import org.openstreetmap.josm.data.osm.OsmPrimitiveType; -import org.openstreetmap.josm.testutils.JOSMTestRules; -import org.openstreetmap.josm.tools.Utils; /** @@ -30,8 +31,40 @@ */ @Test - public void testBuildRequestString() { - 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); + public void testBuildRequestWaysString() { + MultiFetchOverpassObjectReader reader = new MultiFetchOverpassObjectReader(); + reader.append(Arrays.asList(new Way(123), new Way(126), new Way(130))); + String requestString = reader.buildComplexRequestString(); + assertEquals("(way(id:123,126,130);>;);out meta;", requestString); + } + + /** + * Test {@link MultiFetchOverpassObjectReader#buildRequestString} + */ + @Test + public void testBuildRequestRelationsString() { + MultiFetchOverpassObjectReader reader = new MultiFetchOverpassObjectReader(); + reader.append(Arrays.asList(new Relation(123), new Relation(126), new Relation(130))); + reader.setRecurseDownRelations(true); + String requestString = reader.buildComplexRequestString(); + assertEquals("relation(id:123,126,130);>>;out meta;", requestString); + reader.setRecurseDownRelations(false); + requestString = reader.buildComplexRequestString(); + assertEquals("relation(id:123,126,130);out meta;", requestString); + } + + /** + * Test {@link MultiFetchOverpassObjectReader#buildRequestString} + */ + @Test + public void testBuildComplexString() { + MultiFetchOverpassObjectReader reader = new MultiFetchOverpassObjectReader(); + reader.setRecurseDownRelations(true); + reader.append(Arrays.asList(new Relation(123), new Relation(126), new Relation(130), new Way(88), new Way(99), + new Node(1))); + String requestString = reader.buildComplexRequestString(); + assertEquals("(relation(id:123,126,130);>>;(way(id:88,99);>;);node(1););out meta;", requestString); + reader.setRecurseDownRelations(false); + requestString = reader.buildComplexRequestString(); + assertEquals("(relation(id:123,126,130);(way(id:88,99);>;);node(1););out meta;", requestString); }