Ticket #4142: 4142.3.patch

src/org/openstreetmap/josm/actions/downloadtasks/DownloadReferrersTask.java

@@ -121 +121 @@
             return;
         }
 
+        this.children.stream().map(this.parents::getPrimitiveById).forEach(p -> p.setReferrersDownloaded(true));
         DataSetMerger visitor = new DataSetMerger(targetLayer.getDataSet(), parents);
         visitor.merge();
         SwingUtilities.invokeLater(targetLayer::onPostDownloadFromServer);

src/org/openstreetmap/josm/actions/AlignInCircleAction.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.actions;
 
+import static java.util.function.Predicate.not;
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
@@ -271 +272 @@
         }
         fixNodes.addAll(collectNodesWithExternReferrers(ways));
 
-        // Check if one or more nodes are outside of download area
-        if (nodes.stream().anyMatch(Node::isOutsideDownloadArea))
-            throw new InvalidSelection(tr("One or more nodes involved in this action is outside of the downloaded area."));
+        // Check if one or more nodes does not have all parents available
+        if (nodes.stream().anyMatch(not(Node::isReferrersDownloaded)))
+            throw new InvalidSelection(tr("One or more nodes involved in this action may have additional referrers."));
 
 
         if (center == null) {

src/org/openstreetmap/josm/actions/CombineWayAction.java

@@ -284 +284 @@
         for (Way w : selectedWays) {
             final Node[] endnodes = {w.firstNode(), w.lastNode()};
             for (Node n : endnodes) {
-                if (!n.isNew() && n.isOutsideDownloadArea() && !endNodesOutside.add(n)) {
-                    new Notification(tr("Combine ways refused<br>" + "(A shared node is outside of the download area)"))
+                if (!n.isNew() && !n.isReferrersDownloaded() && !endNodesOutside.add(n)) {
+                    new Notification(tr("Combine ways refused<br>" + "(A shared node may have additional referrers)"))
                             .setIcon(JOptionPane.INFORMATION_MESSAGE).show();
                     return;
 

src/org/openstreetmap/josm/actions/DeleteAction.java

@@ -16 +16 @@
 
 import org.openstreetmap.josm.command.DeleteCommand.DeletionCallback;
 import org.openstreetmap.josm.data.osm.DefaultNameFormatter;
+import org.openstreetmap.josm.data.osm.INode;
+import org.openstreetmap.josm.data.osm.IRelation;
+import org.openstreetmap.josm.data.osm.IWay;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationToChildReference;
@@ -103 +106 @@
      */
     public static boolean checkAndConfirmOutlyingDelete(Collection<? extends OsmPrimitive> primitives,
             Collection<? extends OsmPrimitive> ignore) {
+        final boolean nodes = primitives.stream().anyMatch(INode.class::isInstance);
+        final boolean ways = primitives.stream().anyMatch(IWay.class::isInstance);
+        final boolean relations = primitives.stream().anyMatch(IRelation.class::isInstance);
+        final String type;
+        if (nodes && !ways && !relations) {
+            type = tr("You are about to delete nodes which can have other referrers not yet downloaded.");
+        } else if (!nodes && ways && !relations) {
+            type = tr("You are about to delete ways which can have other referrers not yet downloaded.");
+        } else if (!nodes && !ways && relations) {
+            type = tr("You are about to delete relations which can have other referrers not yet downloaded.");
+        } else {
+            // OK. We have multiple types being deleted.
+            type = tr("You are about to delete primitives which can have other referrers not yet downloaded.");
+        }
         return Boolean.TRUE.equals(GuiHelper.runInEDTAndWaitAndReturn(() -> checkAndConfirmOutlyingOperation("delete",
                 tr("Delete confirmation"),
-                tr("You are about to delete nodes which can have other referrers not yet downloaded."
+                tr("{0}"
                         + "<br>"
                         + "This can cause problems because other objects (that you do not see) might use them."
                         + "<br>"
-                        + "Do you really want to delete?"),
+                        + "Do you really want to delete?", type),
                 tr("You are about to delete incomplete objects."
                         + "<br>"
                         + "This will cause problems because you don''t see the real object."

src/org/openstreetmap/josm/actions/JoinAreasAction.java

@@ -672 +672 @@
             commitCommands(marktr("Removed duplicate nodes"));
             // remove now unconnected nodes without tags
             List<Node> toRemove = oldNodes.stream().filter(
-                    n -> (n.isNew() || !n.isOutsideDownloadArea()) && !n.hasKeys() && n.getReferrers().isEmpty())
+                    n -> n.isReferrersDownloaded() && !n.hasKeys() && n.getReferrers().isEmpty())
                     .collect(Collectors.toList());
             if (!toRemove.isEmpty()) {
                 cmds.add(new DeleteCommand(toRemove));

src/org/openstreetmap/josm/actions/SplitWayAction.java

@@ -143 +143 @@
                     .setIcon(JOptionPane.WARNING_MESSAGE)
                     .show();
             return;
+        } else if (!checkAndConfirmOutlyingOperation("splitway", tr("Split way confirmation"),
+                tr("You are about to split a way that may have referrers that are not yet downloaded.")
+                        + "<br/>"
+                        + tr("This can lead to broken relations.") + "<br/>"
+                        + tr("Do you really want to split?"),
+                tr("The selected area is incomplete. Continue?"),
+                applicableWays, null)) {
+            return;
         }
 
         // Finally, applicableWays contains only one perfect way

src/org/openstreetmap/josm/command/Command.java

@@ -227 +227 @@
         for (OsmPrimitive osm : primitives) {
             if (osm.isIncomplete()) {
                 res |= IS_INCOMPLETE;
-            } else if ((res & IS_OUTSIDE) == 0 && (osm.isOutsideDownloadArea()
-                    || (osm instanceof Node && !osm.isNew() && osm.getDataSet() != null && osm.getDataSet().getDataSourceBounds().isEmpty()))
-                            && (ignore == null || !ignore.contains(osm))) {
+            } else if ((res & IS_OUTSIDE) == 0 && !osm.isReferrersDownloaded() && (ignore == null || !ignore.contains(osm))) {
                 res |= IS_OUTSIDE;
             }
         }

src/org/openstreetmap/josm/command/DeleteCommand.java

@@ -433 +433 @@
             primitivesToDelete.addAll(nodesToDelete);
         }
 
-        if (!silent && !callback.checkAndConfirmOutlyingDelete(
-                primitivesToDelete, Utils.filteredCollection(primitivesToDelete, Way.class)))
+        if (!silent && !callback.checkAndConfirmOutlyingDelete(primitivesToDelete, null))
             return null;
 
         Collection<Way> waysToBeChanged = primitivesToDelete.stream()

src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java

@@ -3 +3 @@
 
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.text.MessageFormat;
 import java.time.Instant;
 import java.util.ArrayList;
@@ -130 +133 @@
      */
     protected static final short FLAG_PRESERVED = 1 << 13;
 
+    /**
+     * Determines if the primitive has all of its referrers
+     */
+    protected static final short FLAG_ALL_REFERRERS_DOWNLOADED = 1 << 14;
+
     /**
      * Put several boolean flags to one short int field to save memory.
      * Other bits of this field are used in subclasses.
      */
-    protected volatile short flags = FLAG_VISIBLE;   // visible per default
+    private volatile short flags = FLAG_VISIBLE;   // visible per default
 
     /**
      * The mappaint cache index for this primitive.
@@ -365 +373 @@
         return oldFlags != flags;
     }
 
+    protected void writeObjectCommon(ObjectOutputStream oos) throws IOException {
+        oos.writeLong(id);
+        oos.writeLong(user == null ? -1 : user.getId());
+        oos.writeInt(version);
+        oos.writeInt(changesetId);
+        oos.writeInt(timestamp);
+        oos.writeObject(keys);
+        oos.writeShort(flags);
+    }
+
+    protected void readObjectCommon(ObjectInputStream ois) throws ClassNotFoundException, IOException {
+        id = ois.readLong();
+        final long userId = ois.readLong();
+        user = userId == -1 ? null : User.getById(userId);
+        version = ois.readInt();
+        changesetId = ois.readInt();
+        timestamp = ois.readInt();
+        keys = (String[]) ois.readObject();
+        flags = ois.readShort();
+    }
+
     @Override
     public void setModified(boolean modified) {
         updateFlags(FLAG_MODIFIED, modified);
@@ -380 +409 @@
         return (flags & FLAG_DELETED) != 0;
     }
 
+    @Override
+    public void setReferrersDownloaded(boolean referrersDownloaded) {
+        this.updateFlags(FLAG_ALL_REFERRERS_DOWNLOADED, referrersDownloaded);
+    }
+
     @Override
     public boolean isUndeleted() {
         return (flags & (FLAG_VISIBLE + FLAG_DELETED)) == 0;
@@ -408 +442 @@
         setModified(deleted ^ !isVisible());
     }
 
+    @Override
+    public boolean hasDirectionKeys() {
+        return (flags & FLAG_HAS_DIRECTIONS) != 0;
+    }
+
+    @Override
+    public boolean reversedDirection() {
+        return (flags & FLAG_DIRECTION_REVERSED) != 0;
+    }
+
+    @Override
+    public boolean isTagged() {
+        return (flags & FLAG_TAGGED) != 0;
+    }
+
+    @Override
+    public boolean isAnnotated() {
+        return (flags & FLAG_ANNOTATED) != 0;
+    }
+
+    @Override
+    public boolean isHighlighted() {
+        return (flags & FLAG_HIGHLIGHTED) != 0;
+    }
+
+    @Override
+    public boolean isDisabled() {
+        return (flags & FLAG_DISABLED) != 0;
+    }
+
+    @Override
+    public boolean isDisabledAndHidden() {
+        return ((flags & FLAG_DISABLED) != 0) && ((flags & FLAG_HIDE_IF_DISABLED) != 0);
+    }
+
+    @Override
+    public boolean isPreserved() {
+        return (flags & FLAG_PRESERVED) != 0;
+    }
+
+    @Override
+    public boolean isReferrersDownloaded() {
+        return isNew() || (flags & FLAG_ALL_REFERRERS_DOWNLOADED) != 0;
+    }
+
     /**
      * If set to true, this object is incomplete, which means only the id
      * and type is known (type is the objects instance class)

src/org/openstreetmap/josm/data/osm/IPrimitive.java

@@ -36 +36 @@
      */
     void setModified(boolean modified);
 
+    /**
+     * Set the status of the referrers
+     * @param referrersDownloaded {@code true} if all referrers for this object have been downloaded
+     * @since xxx
+     */
+    default void setReferrersDownloaded(boolean referrersDownloaded) {
+        throw new UnsupportedOperationException(this.getClass().getName() + " does not support referrers status");
+    }
+
     /**
      * Checks if object is known to the server.
      * Replies true if this primitive is either unknown to the server (i.e. its id
@@ -75 +84 @@
      */
     void setDeleted(boolean deleted);
 
+    /**
+     * Determines if this primitive is fully downloaded
+     * @return {@code true} if the primitive is fully downloaded and all parents and children should be available.
+     * {@code false} otherwise.
+     * @since xxx
+     */
+    default boolean isReferrersDownloaded() {
+        return false;
+    }
+
     /**
      * Determines if this primitive is incomplete.
      * @return {@code true} if this primitive is incomplete, {@code false} otherwise

src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

@@ -363 +363 @@
         updateFlags(FLAG_PRESERVED, isPreserved);
     }
 
-    @Override
-    public boolean isDisabled() {
-        return (flags & FLAG_DISABLED) != 0;
-    }
-
-    @Override
-    public boolean isDisabledAndHidden() {
-        return ((flags & FLAG_DISABLED) != 0) && ((flags & FLAG_HIDE_IF_DISABLED) != 0);
-    }
-
-    @Override
-    public boolean isPreserved() {
-        return (flags & FLAG_PRESERVED) != 0;
-    }
-
     @Override
     public boolean isSelectable() {
         // not synchronized -> check disabled twice just to be sure we did not have a race condition.
@@ -492 +477 @@
         }
     }
 
-    @Override
-    public boolean isHighlighted() {
-        return (flags & FLAG_HIGHLIGHTED) != 0;
-    }
-
     /*---------------
      * DIRECTION KEYS
      *---------------*/
@@ -526 +506 @@
         updateFlagsNoLock(FLAG_ANNOTATED, hasKeys() && getWorkInProgressKeys().stream().anyMatch(this::hasKey));
     }
 
-    @Override
-    public boolean isTagged() {
-        return (flags & FLAG_TAGGED) != 0;
-    }
-
-    @Override
-    public boolean isAnnotated() {
-        return (flags & FLAG_ANNOTATED) != 0;
-    }
-
     protected void updateDirectionFlags() {
         boolean hasDirections = false;
         boolean directionReversed = false;
@@ -551 +521 @@
         updateFlagsNoLock(FLAG_HAS_DIRECTIONS, hasDirections);
     }
 
-    @Override
-    public boolean hasDirectionKeys() {
-        return (flags & FLAG_HAS_DIRECTIONS) != 0;
-    }
-
-    @Override
-    public boolean reversedDirection() {
-        return (flags & FLAG_DIRECTION_REVERSED) != 0;
-    }
-
     /*------------
      * Keys handling
      ------------*/
@@ -851 +811 @@
                 throw new DataIntegrityProblemException(
                         tr("Cannot merge primitives with different ids. This id is {0}, the other is {1}", id, other.getId()));
 
+            setIncomplete(other.isIncomplete());
+            super.cloneFrom(other);
             setKeys(other.hasKeys() ? other.getKeys() : null);
-            timestamp = other.timestamp;
             version = other.version;
-            setIncomplete(other.isIncomplete());
-            flags = other.flags;
-            user = other.user;
             changesetId = other.changesetId;
         } finally {
             writeUnlock(locked);

src/org/openstreetmap/josm/data/osm/PrimitiveData.java

@@ -84 +84 @@
 
     private void writeObject(ObjectOutputStream oos) throws IOException {
         // since super class is not Serializable
-        oos.writeLong(id);
-        oos.writeLong(user == null ? -1 : user.getId());
-        oos.writeInt(version);
-        oos.writeInt(changesetId);
-        oos.writeInt(timestamp);
-        oos.writeObject(keys);
-        oos.writeShort(flags);
+        super.writeObjectCommon(oos);
         oos.defaultWriteObject();
     }
 
     private void readObject(ObjectInputStream ois) throws ClassNotFoundException, IOException {
         // since super class is not Serializable
-        id = ois.readLong();
-        final long userId = ois.readLong();
-        user = userId == -1 ? null : User.getById(userId);
-        version = ois.readInt();
-        changesetId = ois.readInt();
-        timestamp = ois.readInt();
-        keys = (String[]) ois.readObject();
-        flags = ois.readShort();
+        super.readObjectCommon(ois);
         ois.defaultReadObject();
     }
 

src/org/openstreetmap/josm/data/validation/tests/BarriersEntrances.java

@@ -26 +26 @@
 
     @Override
     public void visit(Node n) {
-        if (n.hasTag("barrier", "entrance") && !n.isOutsideDownloadArea()) {
+        if (n.hasTag("barrier", "entrance") && n.isReferrersDownloaded()) {
             for (OsmPrimitive p : n.getReferrers()) {
                 if (p.hasKey("barrier")) {
                     return;

src/org/openstreetmap/josm/data/vector/VectorPrimitive.java

@@ -58 +58 @@
         this.highlighted = highlighted;
     }
 
-    @Override
-    public boolean isTagged() {
-        return (flags & FLAG_TAGGED) != 0;
-    }
-
     @Override
     public boolean isAnnotated() {
         return this.getInterestingTags().size() - this.getKeys().size() > 0;

src/org/openstreetmap/josm/io/BoundingBoxDownloader.java

@@ -6 +6 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.SocketException;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 import org.openstreetmap.josm.data.Bounds;
@@ -215 +217 @@
                     ds = parseDataSet(in, progressMonitor.createSubTaskMonitor(1, false));
                 }
             }
+            // From https://wiki.openstreetmap.org/wiki/API_v0.6#Retrieving_map_data_by_bounding_box:_GET_/api/0.6/map,
+            // relations are not recursed up, so they *may* have parent relations.
+            // Nodes inside the download area should have all relations and ways that refer to them.
+            // Ways should have all relations that refer to them and all child nodes, but those child nodes may not
+            //    have their parent referrers.
+            // Relations will have the *first* parent relations downloaded, but those are not split out in the returns.
+            // So we always assume that a relation has referrers that need to be downloaded unless it has no child relations.
+            // Our "full" overpass query doesn't return the same data as a standard download, so we cannot
+            // mark relations with no child relations as fully downloaded *yet*.
+            if (this.considerAsFullDownload()) {
+                final Collection<Bounds> bounds = this.getBounds();
+                // We cannot use OsmPrimitive#isOutsideDownloadArea yet since some download methods haven't added
+                // the download bounds to the dataset yet. This is specifically the case for overpass downloads.
+                ds.getNodes().stream().filter(n -> bounds.stream().anyMatch(b -> b.contains(n)))
+                        .forEach(i -> i.setReferrersDownloaded(true));
+                ds.getWays().forEach(i -> i.setReferrersDownloaded(true));
+            }
             return ds;
         } catch (OsmTransferException e) {
             throw e;
@@ -279 +298 @@
         return true;
     }
 
+    /**
+     * Get the bounds for this downloader
+     * @return The bounds for this downloader
+     * @since xxx
+     */
+    protected Collection<Bounds> getBounds() {
+        return Collections.singleton(new Bounds(this.lat1, this.lon1, this.lat2, this.lon2));
+    }
 }

src/org/openstreetmap/josm/io/OverpassDownloadReader.java

@@ -13 +13 @@
 import java.time.ZoneOffset;
 import java.time.format.DateTimeParseException;
 import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.EnumMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Objects;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
@@ -406 +409 @@
         } else {
             // add bounds if necessary (note that Overpass API does not return bounds in the response XML)
             if (ds != null && ds.getDataSources().isEmpty() && overpassQuery.contains("{{bbox}}")) {
-                if (crosses180th) {
-                    Bounds bounds = new Bounds(lat1, lon1, lat2, 180.0);
-                    DataSource src = new DataSource(bounds, getBaseUrl());
-                    ds.addDataSource(src);
-
-                    bounds = new Bounds(lat1, -180.0, lat2, lon2);
-                    src = new DataSource(bounds, getBaseUrl());
-                    ds.addDataSource(src);
-                } else {
-                    Bounds bounds = new Bounds(lat1, lon1, lat2, lon2);
-                    DataSource src = new DataSource(bounds, getBaseUrl());
-                    ds.addDataSource(src);
-                }
+                getBounds().forEach(bounds -> ds.addDataSource(new DataSource(bounds, getBaseUrl())));
             }
             return ds;
         }
@@ -440 +431 @@
     public boolean considerAsFullDownload() {
         return overpassQuery.equals(OverpassDownloadSource.FULL_DOWNLOAD_QUERY);
     }
+
+    @Override
+    protected Collection<Bounds> getBounds() {
+        if (this.overpassQuery.contains("{{bbox}}")) {
+            if (crosses180th) {
+                return Set.of(new Bounds(lat1, lon1, lat2, 180.0),
+                        new Bounds(lat1, -180.0, lat2, lon2));
+            } else {
+                return Collections.singleton(new Bounds(lat1, lon1, lat2, lon2));
+            }
+        }
+        return Collections.emptySet();
+    }
 }