Index: src/org/openstreetmap/josm/actions/ValidateAction.java =================================================================== --- src/org/openstreetmap/josm/actions/ValidateAction.java (revision 18952) +++ src/org/openstreetmap/josm/actions/ValidateAction.java (working copy) @@ -12,7 +12,6 @@ import org.openstreetmap.josm.data.validation.OsmValidator; import org.openstreetmap.josm.data.validation.Test; import org.openstreetmap.josm.data.validation.ValidationTask; -import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.MapFrame; import org.openstreetmap.josm.tools.Shortcut; @@ -70,8 +69,6 @@ selection = getLayerManager().getActiveDataSet().allNonDeletedPrimitives(); lastSelection = null; } else { - AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor(); - selection = v.visit(selection); lastSelection = selection; } } else { Index: src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java =================================================================== --- src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java (revision 18952) +++ src/org/openstreetmap/josm/actions/upload/ValidateUploadHook.java (working copy) @@ -6,6 +6,7 @@ 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; @@ -17,7 +18,6 @@ import org.openstreetmap.josm.data.validation.OsmValidator; import org.openstreetmap.josm.data.validation.TestError; import org.openstreetmap.josm.data.validation.ValidationTask; -import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor; import org.openstreetmap.josm.gui.ExtendedDialog; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.dialogs.validator.ValidatorTreePanel; @@ -45,9 +45,9 @@ @Override public boolean checkUpload(APIDataSet apiDataSet) { AtomicBoolean returnCode = new AtomicBoolean(); - AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor(); - v.visit(apiDataSet.getPrimitivesToAdd()); - Collection visited = v.visit(apiDataSet.getPrimitivesToUpdate()); + Collection toCheck = new HashSet<>(); + toCheck.addAll(apiDataSet.getPrimitivesToAdd()); + toCheck.addAll(apiDataSet.getPrimitivesToUpdate()); OsmValidator.initializeTests(); new ValidationTask(errors -> { if (errors.stream().allMatch(TestError::isIgnored)) { @@ -58,7 +58,7 @@ // of the progress monitor. GuiHelper.runInEDTAndWait(() -> returnCode.set(displayErrorScreen(errors))); } - }, null, OsmValidator.getEnabledTests(true), visited, null, true).run(); + }, null, OsmValidator.getEnabledTests(true), toCheck, null, true).run(); return returnCode.get(); } Index: src/org/openstreetmap/josm/data/validation/ValidationTask.java =================================================================== --- src/org/openstreetmap/josm/data/validation/ValidationTask.java (revision 18952) +++ src/org/openstreetmap/josm/data/validation/ValidationTask.java (working copy) @@ -6,6 +6,7 @@ import java.awt.GraphicsEnvironment; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -12,8 +13,10 @@ import javax.swing.JOptionPane; +import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper; +import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.MapFrame; import org.openstreetmap.josm.gui.Notification; @@ -30,7 +33,7 @@ public class ValidationTask extends PleaseWaitRunnable { private final Consumer> onFinish; private Collection tests; - private final Collection validatedPrimitives; + private final Collection initialPrimitives; private final Collection formerValidatedPrimitives; private final boolean beforeUpload; private boolean canceled; @@ -71,12 +74,34 @@ progressMonitor != null ? progressMonitor : new PleaseWaitProgressMonitor(tr("Validating")), false /*don't ignore exceptions */); this.onFinish = onFinish; - this.validatedPrimitives = validatedPrimitives; + this.initialPrimitives = validatedPrimitives; this.formerValidatedPrimitives = formerValidatedPrimitives; this.tests = tests; this.beforeUpload = beforeUpload; } + /** + * Create extended list + * - add child objects because MapCss tests may need them to work properly + * - add parent objects of modified nodes as they may mean a geometry change (see #23397) + * @param primitives the primitives + * @return extended list of primitives + */ + private static Collection extendList(Collection primitives) { + Collection extendedList = new HashSet<>(primitives); + for (OsmPrimitive p : primitives) { + if (p instanceof Node && p.isModified() && !p.isNew()) { + for (OsmPrimitive parent : p.getReferrers()) { + if (!parent.isDeleted()) { + extendedList.add(parent); + } + } + } + } + AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor(); + return v.visit(extendedList); + } + protected ValidationTask(ProgressMonitor progressMonitor, Collection tests, Collection validatedPrimitives, @@ -122,8 +147,12 @@ protected void realRun() { if (Utils.isEmpty(tests)) return; + int testCounter = 0; + final boolean isPartial = this.beforeUpload || formerValidatedPrimitives != null; + + Collection validatedPrimitives = isPartial ? extendList(initialPrimitives) : initialPrimitives; getProgressMonitor().setTicksCount(tests.size() * validatedPrimitives.size()); - int testCounter = 0; + for (Test test : tests) { if (canceled) return; @@ -131,7 +160,7 @@ getProgressMonitor().setCustomText(tr("Test {0}/{1}: Starting {2}", testCounter, tests.size(), test.getName())); test.setBeforeUpload(this.beforeUpload); // Pre-upload checks only run on a partial selection. - test.setPartialSelection(this.beforeUpload || formerValidatedPrimitives != null); + test.setPartialSelection(isPartial); test.startTest(getProgressMonitor().createSubTaskMonitor(validatedPrimitives.size(), false)); test.visit(validatedPrimitives); test.endTest(); Index: src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 18952) +++ src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (working copy) @@ -6,6 +6,7 @@ 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,6 +17,8 @@ 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,6 +84,7 @@ private final Map> cellSegments = new HashMap<>(1000); /** The already detected ways in error */ private final Map, List> seenWays = new HashMap<>(50); + private final Set waysToTest = new HashSet<>(); protected final int code; @@ -305,9 +309,28 @@ @Override public void endTest() { - super.endTest(); + final Collection selection; + if (this instanceof SelfCrossing || !partialSelection) { + selection = waysToTest; + } else { + selection = new HashSet<>(); + DataSet ds = OsmDataManager.getInstance().getActiveDataSet(); + if (ds != null) { + 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(); + waysToTest.clear(); + super.endTest(); } static boolean isCoastline(OsmPrimitive w) { @@ -344,6 +367,10 @@ @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,6 +509,7 @@ CheckParameterUtil.ensureParameterNotNull(way, "way"); SelfCrossing test = new SelfCrossing(); test.visit(way); + test.endTest(); return !test.getErrors().isEmpty(); } } Index: src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java (revision 18952) +++ src/org/openstreetmap/josm/data/validation/tests/DuplicateWay.java (working copy) @@ -7,6 +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,6 +104,7 @@ /** Set of known hashcodes for list of coordinates **/ private Set knownHashCodes; + private List waysToCheck; /** * Constructor @@ -115,6 +117,7 @@ @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,7 +125,22 @@ @Override public void endTest() { - super.endTest(); + if (partialSelection && !waysToCheck.isEmpty()) { + // make sure that we have the error candidates even if not selected + Set 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 duplicated : ways.values()) { if (duplicated.size() > 1) { TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_WAY) @@ -165,6 +183,8 @@ ways = null; waysNoTags = null; knownHashCodes = null; + waysToCheck = null; + super.endTest(); } /** @@ -181,6 +201,13 @@ public void visit(Way w) { if (!w.isUsable()) return; + if (partialSelection) + waysToCheck.add(w); + else + checkWay(w); + } + + private void checkWay(Way w) { List 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()) { Index: src/org/openstreetmap/josm/data/validation/util/AggregatePrimitivesVisitor.java =================================================================== --- src/org/openstreetmap/josm/data/validation/util/AggregatePrimitivesVisitor.java (revision 18952) +++ src/org/openstreetmap/josm/data/validation/util/AggregatePrimitivesVisitor.java (working copy) @@ -14,7 +14,6 @@ /** * A visitor that aggregates all primitives it visits. *

- * The primitives are sorted according to their type: first nodes, then ways. * * @author frsantos */ @@ -24,7 +23,7 @@ /** * Visits a collection of primitives - * @param data The collection of primitives + * @param data The collection of primitives in no specific order. * @return The aggregated primitives */ public Collection visit(Collection data) {