Ticket #23397: 23397-a2.patch

src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java

@@ -6 +6 @@
 import java.awt.Dimension;
 import java.awt.GridBagLayout;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -13 +14 @@
 import javax.swing.JScrollPane;
 
 import org.openstreetmap.josm.data.APIDataSet;
+import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.validation.OsmValidator;
 import org.openstreetmap.josm.data.validation.TestError;
@@ -45 +47 @@
     @Override
     public boolean checkUpload(APIDataSet apiDataSet) {
         AtomicBoolean returnCode = new AtomicBoolean();
+        Collection<OsmPrimitive> initialList = new HashSet<>(apiDataSet.getPrimitivesToAdd());
+        initialList.addAll(apiDataSet.getPrimitivesToUpdate());
+        // see #23397
+        // create extended list with parent objects of modified nodes as they may mean a geometry change
+        // maybe find a better place to do this?
+        Collection<OsmPrimitive> extendedList = new HashSet<>(initialList);
+        for (OsmPrimitive p : initialList) {
+            if (p instanceof Node && !p.isNew()) {
+                for (OsmPrimitive parent : p.getReferrers()) {
+                    if (!parent.isDeleted()) {
+                        extendedList.add(parent);
+                    }
+                }
+            }
+        }
         AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor();
-        v.visit(apiDataSet.getPrimitivesToAdd());
-        Collection<OsmPrimitive> visited = v.visit(apiDataSet.getPrimitivesToUpdate());
+        Collection<OsmPrimitive> visited = v.visit(extendedList);
         OsmValidator.initializeTests();
         new ValidationTask(errors -> {
             if (errors.stream().allMatch(TestError::isIgnored)) {

src/org/openstreetmap/josm/data/validation/Test.java

@@ -6 +6 @@
 import java.awt.GridBagConstraints;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
@@ -383 +385 @@
     public Object getSource() {
         return "Java: " + this.getClass().getName();
     }
+
+    /**
+     * Filter the list of errors, remove all which do not concern the given list of primitives
+     * @param given the list of primitives
+     * @since xxx
+     */
+    public void removeIrrelevantErrors(Collection<? extends OsmPrimitive> given) {
+        if (errors == null || errors.isEmpty())
+            return;
+        // filter errors for those which are needed, don't show errors for objects which were not in the selection
+        final Set<? extends OsmPrimitive> relevant;
+        if (given instanceof Set) {
+            relevant = (Set<? extends OsmPrimitive>) given;
+        } else {
+            relevant = new HashSet<>(given);
+        }
+        errors.removeIf(e -> !e.isConcerned(relevant));
+    }
 }

src/org/openstreetmap/josm/data/validation/TestError.java

@@ -12 +12 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeSet;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
@@ -666 +667 @@
                 ", code=" + code + ", message=" + message + ']';
     }
 
+    /**
+     * Check if any of the primitives in this error occurs in the given set of primitives.
+     * @param given the set of primitives
+     * @return true if any of the primitives in this error occurs in the given set of primitives, else false
+     * @since xxx
+     */
+    public boolean isConcerned(Set<? extends OsmPrimitive> given) {
+        for (OsmPrimitive p : getPrimitives()) {
+            if (given.contains(p)) {
+                return true;
+            }
+        }
+        return false;
+    }
 }

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

@@ -6 +6 @@
 import java.awt.geom.Point2D;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -16 +17 @@
 
 import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.coor.ILatLon;
+import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.OsmDataManager;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.OsmUtils;
 import org.openstreetmap.josm.data.osm.Relation;
@@ -81 +84 @@
     private final Map<Point2D, List<WaySegment>> cellSegments = new HashMap<>(1000);
     /** The already detected ways in error */
     private final Map<List<Way>, List<WaySegment>> seenWays = new HashMap<>(50);
+    private final Set<Way> waysToTest = new HashSet<>();
 
     protected final int code;
+    private DataSet ds;
 
     /**
      * General crossing ways test.
@@ -305 +310 @@
 
     @Override
     public void endTest() {
-        super.endTest();
+        ds = OsmDataManager.getInstance().getActiveDataSet();
+        if (ds == null || waysToTest.isEmpty())
+            return;
+        final Collection<Way> selection;
+        if (this instanceof SelfCrossing || !partialSelection) {
+            selection = waysToTest;
+        } else {
+            selection = new HashSet<>();
+            for (Way w: waysToTest) {
+                selection.addAll(ds.searchWays(w.getBBox()));
+            }
+        }
+        for (Way w : selection) {
+            if (!w.isDeleted() && isPrimitiveUsable(w)) {
+                testWay(w);
+            }
+        }
+        // free storage
         cellSegments.clear();
         seenWays.clear();
+        if (partialSelection) {
+            removeIrrelevantErrors(waysToTest);
+        }
+        super.endTest();
+        waysToTest.clear();
+        ds = null;
     }
 
     static boolean isCoastline(OsmPrimitive w) {
@@ -344 +372 @@
 
     @Override
     public void visit(Way w) {
+        waysToTest.add(w);
+    }
+
+    private void testWay(Way w) {
         boolean findSelfCrossingOnly = this instanceof SelfCrossing;
         if (findSelfCrossingOnly) {
             // free memory, we are not interested in previous ways
@@ -482 +514 @@
         CheckParameterUtil.ensureParameterNotNull(way, "way");
         SelfCrossing test = new SelfCrossing();
         test.visit(way);
+        test.endTest();
         return !test.getErrors().isEmpty();
     }
 }

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

@@ -7 +7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -103 +104 @@
 
     /** Set of known hashcodes for list of coordinates **/
     private Set<Integer> knownHashCodes;
+    private List<Way> waysToCheck;
 
     /**
      * Constructor
@@ -115 +117 @@
     @Override
     public void startTest(ProgressMonitor monitor) {
         super.startTest(monitor);
+        waysToCheck = new ArrayList<>();
         ways = new MultiMap<>(1000);
         waysNoTags = new MultiMap<>(1000);
         knownHashCodes = new HashSet<>(1000);
@@ -122 +125 @@
 
     @Override
     public void endTest() {
-        super.endTest();
+        if (partialSelection && !waysToCheck.isEmpty()) {
+            // make sure that we have the error candidates even if not selected
+            Set<Way> extended = new LinkedHashSet<>(waysToCheck);
+            for (Way w : waysToCheck) {
+                // select a node, anyone can be used but a middle node is less likely to have many parent ways
+                final Node n = w.getNode(w.getNodesCount()/2);
+                // check the ways which might be in the same position
+                for (Way other : n.getParentWays()) {
+                    if (other != w && !other.isDeleted() && other.isUsable()
+                            && other.getNodesCount() == w.getNodesCount())
+                        extended.add(other);
+                }
+            }
+            extended.forEach(this::checkWay);
+        }
+
         for (Set<OsmPrimitive> duplicated : ways.values()) {
             if (duplicated.size() > 1) {
                 TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_WAY)
@@ -162 +180 @@
                 errors.add(testError);
             }
         }
+
+        if (partialSelection) {
+            removeIrrelevantErrors(waysToCheck);
+        }
+
         ways = null;
         waysNoTags = null;
         knownHashCodes = null;
+        waysToCheck = null;
+        super.endTest();
     }
 
     /**
@@ -181 +206 @@
     public void visit(Way w) {
         if (!w.isUsable())
             return;
+        if (partialSelection)
+            waysToCheck.add(w);
+        else
+            checkWay(w);
+    }
+
+    private void checkWay(Way w) {
         List<LatLon> wLat = getOrderedNodes(w);
         // If this way has not direction-dependant keys, make sure the list is ordered the same for all ways (fix #8015)
         if (!w.hasDirectionKeys()) {