Ticket #23397: 23397-beta3.patch

File 23397-beta3.patch, 20.4 KB (added by GerdP, 4 months ago)

  • src/org/openstreetmap/josm/actions/ValidateAction.java

     
    1212import org.openstreetmap.josm.data.validation.OsmValidator;
    1313import org.openstreetmap.josm.data.validation.Test;
    1414import org.openstreetmap.josm.data.validation.ValidationTask;
    15 import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor;
    1615import org.openstreetmap.josm.gui.MainApplication;
    1716import org.openstreetmap.josm.gui.MapFrame;
    1817import org.openstreetmap.josm.tools.Shortcut;
    … …  
    7069                selection = getLayerManager().getActiveDataSet().allNonDeletedPrimitives();
    7170                lastSelection = null;
    7271            } else {
    73                 AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor();
    74                 selection = v.visit(selection);
    7572                lastSelection = selection;
    7673            }
    7774        } else {

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

     
    66import java.awt.Dimension;
    77import java.awt.GridBagLayout;
    88import java.util.Collection;
     9import java.util.HashSet;
    910import java.util.List;
    1011import java.util.concurrent.atomic.AtomicBoolean;
    1112
    … …  
    1718import org.openstreetmap.josm.data.validation.OsmValidator;
    1819import org.openstreetmap.josm.data.validation.TestError;
    1920import org.openstreetmap.josm.data.validation.ValidationTask;
    20 import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor;
    2121import org.openstreetmap.josm.gui.ExtendedDialog;
    2222import org.openstreetmap.josm.gui.MainApplication;
    2323import org.openstreetmap.josm.gui.dialogs.validator.ValidatorTreePanel;
    … …  
    4545    @Override
    4646    public boolean checkUpload(APIDataSet apiDataSet) {
    4747        AtomicBoolean returnCode = new AtomicBoolean();
    48         AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor();
    49         v.visit(apiDataSet.getPrimitivesToAdd());
    50         Collection<OsmPrimitive> visited = v.visit(apiDataSet.getPrimitivesToUpdate());
     48        Collection<OsmPrimitive> toCheck = new HashSet<>();
     49        toCheck.addAll(apiDataSet.getPrimitivesToAdd());
     50        toCheck.addAll(apiDataSet.getPrimitivesToUpdate());
    5151        OsmValidator.initializeTests();
    5252        new ValidationTask(errors -> {
    5353            if (errors.stream().allMatch(TestError::isIgnored)) {
    … …  
    5858                // of the progress monitor.
    5959                GuiHelper.runInEDTAndWait(() -> returnCode.set(displayErrorScreen(errors)));
    6060            }
    61         }, null, OsmValidator.getEnabledTests(true), visited, null, true).run();
     61        }, null, OsmValidator.getEnabledTests(true), toCheck, null, true).run();
    6262
    6363        return returnCode.get();
    6464    }

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

     
    66import java.awt.GridBagConstraints;
    77import java.util.ArrayList;
    88import java.util.Collection;
     9import java.util.HashSet;
    910import java.util.List;
    1011import java.util.Optional;
     12import java.util.Set;
    1113import java.util.function.Predicate;
    1214import java.util.stream.Collectors;
    1315
    … …  
    383385    public Object getSource() {
    384386        return "Java: " + this.getClass().getName();
    385387    }
     388
     389    /**
     390     * Filter the list of errors, remove all which do not concern the given list of primitives
     391     * @param given the list of primitives
     392     * @since xxx
     393     */
     394    public void removeIrrelevantErrors(Collection<? extends OsmPrimitive> given) {
     395        if (errors == null || errors.isEmpty())
     396            return;
     397        // filter errors for those which are needed, don't show errors for objects which were not in the selection
     398        final Set<? extends OsmPrimitive> relevant;
     399        if (given instanceof Set) {
     400            relevant = (Set<? extends OsmPrimitive>) given;
     401        } else {
     402            relevant = new HashSet<>(given);
     403        }
     404        errors.removeIf(e -> !e.isConcerned(relevant));
     405    }
    386406}

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

     
    1212import java.util.List;
    1313import java.util.Locale;
    1414import java.util.Map;
     15import java.util.Set;
    1516import java.util.TreeSet;
    1617import java.util.function.Supplier;
    1718import java.util.stream.Collectors;
    … …  
    6263    private final int uniqueCode;
    6364    /** If this error is selected */
    6465    private boolean selected;
     66    /** If all relevant primitives are known*/
     67    private boolean incompletePrimitives;
    6568    /** Supplying a command to fix the error */
    6669    private final Supplier<Command> fixingCommand;
    6770
    … …  
    8083        private Collection<? extends OsmPrimitive> primitives;
    8184        private Collection<?> highlighted;
    8285        private Supplier<Command> fixingCommand;
     86        private boolean incompletePrimitives;
    8387
    8488        Builder(Test tester, Severity severity, int code) {
    8589            this.tester = tester;
    … …  
    218222        }
    219223
    220224        /**
     225         * Sets a flag that the list of primitives may be incomplete. See #23397
     226         *
     227         * @return {@code this}
     228         */
     229        public Builder imcompletePrimitives() {
     230            this.incompletePrimitives = true;
     231            return this;
     232        }
     233
     234        /**
    221235         * Sets a supplier to obtain a command to fix the error.
    222236         *
    223237         * @param fixingCommand the fix supplier. Can be null
    … …  
    276290        this.code = builder.code;
    277291        this.uniqueCode = builder.uniqueCode;
    278292        this.fixingCommand = builder.fixingCommand;
     293        this.incompletePrimitives = builder.incompletePrimitives;
    279294    }
    280295
    281296    /**
    … …  
    666681                ", code=" + code + ", message=" + message + ']';
    667682    }
    668683
     684    /**
     685     * Check if any of the primitives in this error occurs in the given set of primitives.
     686     * @param given the set of primitives
     687     * @return true if any of the primitives in this error occurs in the given set of primitives, else false
     688     * @since xxx
     689     */
     690    public boolean isConcerned(Set<? extends OsmPrimitive> given) {
     691        if (incompletePrimitives)
     692            return true;
     693        for (OsmPrimitive p : getPrimitives()) {
     694            if (given.contains(p)) {
     695                return true;
     696            }
     697        }
     698        return false;
     699    }
    669700}

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

     
    66import java.awt.GraphicsEnvironment;
    77import java.util.ArrayList;
    88import java.util.Collection;
     9import java.util.HashSet;
    910import java.util.List;
     11import java.util.Set;
    1012import java.util.function.BiConsumer;
    1113import java.util.function.Consumer;
    1214
    1315import javax.swing.JOptionPane;
    1416
     17import org.openstreetmap.josm.data.osm.Node;
    1518import org.openstreetmap.josm.data.osm.OsmPrimitive;
     19import org.openstreetmap.josm.data.osm.Way;
    1620import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
     21import org.openstreetmap.josm.data.validation.util.AggregatePrimitivesVisitor;
    1722import org.openstreetmap.josm.gui.MainApplication;
    1823import org.openstreetmap.josm.gui.MapFrame;
    1924import org.openstreetmap.josm.gui.Notification;
    … …  
    3035public class ValidationTask extends PleaseWaitRunnable {
    3136    private final Consumer<List<TestError>> onFinish;
    3237    private Collection<Test> tests;
    33     private final Collection<OsmPrimitive> validatedPrimitives;
     38    private final Collection<OsmPrimitive> initialPrimitives;
    3439    private final Collection<OsmPrimitive> formerValidatedPrimitives;
    3540    private final boolean beforeUpload;
    3641    private boolean canceled;
    … …  
    7176                progressMonitor != null ? progressMonitor : new PleaseWaitProgressMonitor(tr("Validating")),
    7277                false /*don't ignore exceptions */);
    7378        this.onFinish = onFinish;
    74         this.validatedPrimitives = validatedPrimitives;
     79        this.initialPrimitives = validatedPrimitives;
    7580        this.formerValidatedPrimitives = formerValidatedPrimitives;
    7681        this.tests = tests;
    7782        this.beforeUpload = beforeUpload;
    7883    }
    7984
     85    /**
     86     * Find objects parent objects of given objects which should be checked for changed geometry.
     87     * @param primitives the given objects
     88     * @return a collection of objects with
     89     */
     90    private static Set<OsmPrimitive> getPotentiallyModified(Collection<OsmPrimitive> primitives) {
     91        Set<OsmPrimitive> addedWays = new HashSet<>();
     92        Set<OsmPrimitive> addedRelations = new HashSet<>();
     93        for (OsmPrimitive p : primitives) {
     94            if (!(p instanceof Node && p.isModified() && !p.isNew()))
     95                continue;
     96            for (OsmPrimitive parent : p.getReferrers()) {
     97                if (parent.isDeleted() || parent.isModified() || parent.isNew())
     98                    continue;
     99                if (parent instanceof Way)
     100                    addedWays.add(parent);
     101                else
     102                    addedRelations.add(parent);
     103            }
     104        }
     105        // allow to find invalid multipolygon relations caused by moved nodes
     106        OsmPrimitive.getParentRelations(addedWays).stream().filter(r -> r.isMultipolygon())
     107                .forEach(addedRelations::add);
     108        HashSet<OsmPrimitive> extendedSet = new HashSet<>();
     109        extendedSet.addAll(addedWays);
     110        extendedSet.addAll(addedRelations);
     111        return extendedSet;
     112
     113    }
    80114    protected ValidationTask(ProgressMonitor progressMonitor,
    81115            Collection<Test> tests,
    82116            Collection<OsmPrimitive> validatedPrimitives,
    … …  
    122156    protected void realRun() {
    123157        if (Utils.isEmpty(tests))
    124158            return;
     159        int testCounter = 0;
     160        final boolean isPartial = this.beforeUpload || formerValidatedPrimitives != null;
     161        Set<OsmPrimitive> filter = null;
     162        Collection<OsmPrimitive> validatedPrimitives = initialPrimitives;
     163        if (isPartial) {
     164            Set<OsmPrimitive> other = getPotentiallyModified(initialPrimitives);
     165            HashSet<OsmPrimitive> extendedSet = new HashSet<>();
     166            AggregatePrimitivesVisitor v = new AggregatePrimitivesVisitor();
     167            extendedSet.addAll(v.visit(initialPrimitives));
     168            extendedSet.addAll(other);
     169            validatedPrimitives = extendedSet;
     170            filter = new HashSet<>(initialPrimitives);
     171            filter.addAll(other);
     172        }
    125173        getProgressMonitor().setTicksCount(tests.size() * validatedPrimitives.size());
    126         int testCounter = 0;
     174
    127175        for (Test test : tests) {
    128176            if (canceled)
    129177                return;
    … …  
    131179            getProgressMonitor().setCustomText(tr("Test {0}/{1}: Starting {2}", testCounter, tests.size(), test.getName()));
    132180            test.setBeforeUpload(this.beforeUpload);
    133181            // Pre-upload checks only run on a partial selection.
    134             test.setPartialSelection(this.beforeUpload || formerValidatedPrimitives != null);
     182            test.setPartialSelection(isPartial);
    135183            test.startTest(getProgressMonitor().createSubTaskMonitor(validatedPrimitives.size(), false));
    136184            test.visit(validatedPrimitives);
    137185            test.endTest();
     186            if (isPartial) {
     187                // #23397: remove errors for objects which were not in the initial list of primitives
     188                test.removeIrrelevantErrors(filter);
     189            }
     190
    138191            errors.addAll(test.getErrors());
    139192            if (this.testConsumer != null) {
    140193                this.testConsumer.accept(this, test);

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

     
    66import java.awt.geom.Point2D;
    77import java.util.ArrayList;
    88import java.util.Arrays;
     9import java.util.Collection;
    910import java.util.HashMap;
    1011import java.util.HashSet;
    1112import java.util.List;
    … …  
    1617
    1718import org.openstreetmap.josm.data.coor.EastNorth;
    1819import org.openstreetmap.josm.data.coor.ILatLon;
     20import org.openstreetmap.josm.data.osm.DataSet;
     21import org.openstreetmap.josm.data.osm.OsmDataManager;
    1922import org.openstreetmap.josm.data.osm.OsmPrimitive;
    2023import org.openstreetmap.josm.data.osm.OsmUtils;
    2124import org.openstreetmap.josm.data.osm.Relation;
    … …  
    8184    private final Map<Point2D, List<WaySegment>> cellSegments = new HashMap<>(1000);
    8285    /** The already detected ways in error */
    8386    private final Map<List<Way>, List<WaySegment>> seenWays = new HashMap<>(50);
     87    private final Set<Way> waysToTest = new HashSet<>();
    8488
    8589    protected final int code;
    8690
    … …  
    305309
    306310    @Override
    307311    public void endTest() {
    308         super.endTest();
     312        final Collection<Way> selection;
     313        if (this instanceof SelfCrossing || !partialSelection) {
     314            selection = waysToTest;
     315        } else {
     316            selection = new HashSet<>();
     317            DataSet ds = OsmDataManager.getInstance().getActiveDataSet();
     318            if (ds != null) {
     319                for (Way w: waysToTest) {
     320                    selection.addAll(ds.searchWays(w.getBBox()));
     321                }
     322            }
     323        }
     324        for (Way w : selection) {
     325            if (!w.isDeleted() && isPrimitiveUsable(w)) {
     326                testWay(w);
     327            }
     328        }
     329        // free storage
    309330        cellSegments.clear();
    310331        seenWays.clear();
     332        waysToTest.clear();
     333        super.endTest();
    311334    }
    312335
    313336    static boolean isCoastline(OsmPrimitive w) {
    … …  
    344367
    345368    @Override
    346369    public void visit(Way w) {
     370        waysToTest.add(w);
     371    }
     372
     373    private void testWay(Way w) {
    347374        boolean findSelfCrossingOnly = this instanceof SelfCrossing;
    348375        if (findSelfCrossingOnly) {
    349376            // free memory, we are not interested in previous ways
    … …  
    482509        CheckParameterUtil.ensureParameterNotNull(way, "way");
    483510        SelfCrossing test = new SelfCrossing();
    484511        test.visit(way);
     512        test.endTest();
    485513        return !test.getErrors().isEmpty();
    486514    }
    487515}

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

     
    77import java.util.Collection;
    88import java.util.Collections;
    99import java.util.HashSet;
     10import java.util.LinkedHashSet;
    1011import java.util.LinkedList;
    1112import java.util.List;
    1213import java.util.Map;
    … …  
    103104
    104105    /** Set of known hashcodes for list of coordinates **/
    105106    private Set<Integer> knownHashCodes;
     107    private List<Way> waysToCheck;
    106108
    107109    /**
    108110     * Constructor
    … …  
    115117    @Override
    116118    public void startTest(ProgressMonitor monitor) {
    117119        super.startTest(monitor);
     120        waysToCheck = new ArrayList<>();
    118121        ways = new MultiMap<>(1000);
    119122        waysNoTags = new MultiMap<>(1000);
    120123        knownHashCodes = new HashSet<>(1000);
    … …  
    122125
    123126    @Override
    124127    public void endTest() {
    125         super.endTest();
     128        if (partialSelection && !waysToCheck.isEmpty()) {
     129            // make sure that we have the error candidates even if not selected
     130            Set<Way> extended = new LinkedHashSet<>(waysToCheck);
     131            for (Way w : waysToCheck) {
     132                // select a node, anyone can be used but a middle node is less likely to have many parent ways
     133                final Node n = w.getNode(w.getNodesCount()/2);
     134                // check the ways which might be in the same position
     135                for (Way other : n.getParentWays()) {
     136                    if (other != w && !other.isDeleted() && other.isUsable()
     137                            && other.getNodesCount() == w.getNodesCount())
     138                        extended.add(other);
     139                }
     140            }
     141            extended.forEach(this::checkWay);
     142        }
     143
    126144        for (Set<OsmPrimitive> duplicated : ways.values()) {
    127145            if (duplicated.size() > 1) {
    128146                TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_WAY)
    … …  
    165183        ways = null;
    166184        waysNoTags = null;
    167185        knownHashCodes = null;
     186        waysToCheck = null;
     187        super.endTest();
    168188    }
    169189
    170190    /**
    … …  
    181201    public void visit(Way w) {
    182202        if (!w.isUsable())
    183203            return;
     204        if (partialSelection)
     205            waysToCheck.add(w);
     206        else
     207            checkWay(w);
     208    }
     209
     210    private void checkWay(Way w) {
    184211        List<LatLon> wLat = getOrderedNodes(w);
    185212        // If this way has not direction-dependant keys, make sure the list is ordered the same for all ways (fix #8015)
    186213        if (!w.hasDirectionKeys()) {

  • src/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerRule.java

     
    3333import org.openstreetmap.josm.gui.mappaint.Keyword;
    3434import org.openstreetmap.josm.gui.mappaint.MultiCascade;
    3535import org.openstreetmap.josm.gui.mappaint.mapcss.Condition;
     36import org.openstreetmap.josm.gui.mappaint.mapcss.ConditionFactory.ClassCondition;
    3637import org.openstreetmap.josm.gui.mappaint.mapcss.Expression;
    3738import org.openstreetmap.josm.gui.mappaint.mapcss.Instruction;
    3839import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSRule;
    … …  
    377378                        res.add(errorBuilder.primitives(p, (OsmPrimitive) c).build());
    378379                    }
    379380                }
     381            } else if (env.parent != null) {
     382                boolean imcompletePrimitives = false;
     383                if (matchingSelector instanceof Selector.ChildOrParentSelector) {
     384                    Selector right = ((Selector.ChildOrParentSelector) matchingSelector).right;
     385                    if (right.getConditions().stream().anyMatch(ClassCondition.class::isInstance)) {
     386                        // see #23397
     387                        // TODO: find a way to collect all and only those parent objects which triggered this error
     388                        imcompletePrimitives = true;
     389                    }
     390                }
     391                if (imcompletePrimitives)
     392                    res.add(errorBuilder.primitives(p).highlight(p).imcompletePrimitives().build());
     393                else
     394                    res.add(errorBuilder.primitives(p, (OsmPrimitive) env.parent).highlight(p).build());
    380395            } else {
    381396                res.add(errorBuilder.primitives(p).build());
    382397            }

  • src/org/openstreetmap/josm/data/validation/util/AggregatePrimitivesVisitor.java

     
    1414/**
    1515 * A visitor that aggregates all primitives it visits.
    1616 * <p>
    17  * The primitives are sorted according to their type: first nodes, then ways.
    1817 *
    1918 * @author frsantos
    2019 */
    … …  
    2423
    2524    /**
    2625     * Visits a collection of primitives
    27      * @param data The collection of primitives
     26     * @param data The collection of primitives in no specific order.
    2827     * @return The aggregated primitives
    2928     */
    3029    public Collection<OsmPrimitive> visit(Collection<OsmPrimitive> data) {