Changeset 18960 in josm for trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java

Timestamp:

2024-01-30T09:04:11+01:00 (18 months ago)

Author:

GerdP

Message:

fix #23397: Improve the results of partial validations

• pass also parent ways and relations of uploaded/selected objects to the testers, child objects are already added, this allows to find e.g. overlapping polygons problems with tags in members of relations
• let CrossingWays find a problem if at least one of the crossing ways is in the partial selection.
• let DuplicatWays find a problem if at least one of the duplicated ways is in the partial selection
• add code to filter the detected issues so that those issues which are clearly not related to the original list of objects are removed. A few issues from mapcss tests may remain.
• add new preference validator.partial.add.parents to disable the addition of parent objects, default is enabled
• add new preference validator.partial.removeIrrelevant to disable the filtering of irrelevant errors, default is enabled
• duplicated code to call AggregatePrimitivesVisitor was moved to ValidationTask

File:

• trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java (modified) (7 diffs)

trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java

@@ -7 +7 @@
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
@@ -14 +17 @@
 
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
+import org.openstreetmap.josm.data.osm.Way;
 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;
@@ -31 +36 @@
     private final Consumer<List<TestError>> onFinish;
     private Collection<Test> tests;
-    private final Collection<OsmPrimitive> validatedPrimitives;
+    private final Collection<OsmPrimitive> initialPrimitives;
     private final Collection<OsmPrimitive> formerValidatedPrimitives;
     private final boolean beforeUpload;
@@ -72 +77 @@
                 false /*don't ignore exceptions */);
         this.onFinish = onFinish;
-        this.validatedPrimitives = validatedPrimitives;
+        this.initialPrimitives = validatedPrimitives;
         this.formerValidatedPrimitives = formerValidatedPrimitives;
         this.tests = tests;
@@ -78 +83 @@
     }
 
+    /**
+     * Find objects parent objects of given objects which should be checked for geometry problems
+     * or mismatches between child tags and parent tags.
+     * @param primitives the given objects
+     * @return the collection of relevant parent objects
+     */
+    private static Set<OsmPrimitive> getRelevantParents(Collection<OsmPrimitive> primitives) {
+        Set<OsmPrimitive> addedWays = new HashSet<>();
+        Set<OsmPrimitive> addedRelations = new HashSet<>();
+        for (OsmPrimitive p : primitives) {
+            for (OsmPrimitive parent : p.getReferrers()) {
+                if (parent.isDeleted())
+                    continue;
+                if (parent instanceof Way)
+                    addedWays.add(parent);
+                else
+                    addedRelations.add(parent);
+            }
+        }
+
+        // allow to find invalid multipolygon relations caused by moved nodes
+        OsmPrimitive.getParentRelations(addedWays).stream().filter(r -> r.isMultipolygon() && !r.isDeleted())
+                .forEach(addedRelations::add);
+        HashSet<OsmPrimitive> extendedSet = new HashSet<>();
+        extendedSet.addAll(addedWays);
+        extendedSet.addAll(addedRelations);
+        return extendedSet;
+
+    }
     protected ValidationTask(ProgressMonitor progressMonitor,
             Collection<Test> tests,
@@ -123 +157 @@
         if (Utils.isEmpty(tests))
             return;
+        int testCounter = 0;
+        final boolean isPartial = this.beforeUpload || formerValidatedPrimitives != null;
+        Set<OsmPrimitive> filter = null;
+        Collection<OsmPrimitive> validatedPrimitives = initialPrimitives;
+        if (isPartial) {
+            Set<OsmPrimitive> other = Collections.emptySet();
+            if (Boolean.TRUE.equals(ValidatorPrefHelper.PREF_ADD_PARENTS.get())) {
+                other = getRelevantParents(initialPrimitives);
+            }
+            HashSet<OsmPrimitive> extendedSet = new HashSet<>();
+            AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor();
+            extendedSet.addAll(v.visit(initialPrimitives));
+            extendedSet.addAll(other);
+            validatedPrimitives = extendedSet;
+            filter = new HashSet<>(initialPrimitives);
+            filter.addAll(other);
+        }
         getProgressMonitor().setTicksCount(tests.size() * validatedPrimitives.size());
-        int testCounter = 0;
+
         for (Test test : tests) {
             if (canceled)
@@ -132 +183 @@
             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();
+            if (isPartial && Boolean.TRUE.equals(ValidatorPrefHelper.PREF_REMOVE_IRRELEVANT.get())) {
+                // #23397: remove errors for objects which were not in the initial list of primitives
+                test.removeIrrelevantErrors(filter);
+            }
+
             errors.addAll(test.getErrors());
             if (this.testConsumer != null) {