Changeset 17400 in josm


Ignore:
Timestamp:
2020-12-10T17:38:18+01:00 (3 months ago)
Author:
GerdP
Message:

fix #20121: Confusing handling of water objects in CrossingWays

  • report all self crossing ways with the same message
  • ignore corssing water areas because they are reported by mapcss geometry tests
  • don't ignore crossing between water areas and highway, railway, barrier, or building
  • remove redundant and missleading checks in CrossingWays.Ways.ignoreWaySegmentCombination()
  • add unit test for coverage and test of these changes
  • fix @since
Location:
trunk
Files:
1 added
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java

    r17393 r17400  
    114114        boolean ignoreWaySegmentCombination(Way w1, Way w2) {
    115115            if (w1 == w2)
    116                 return false;
    117             if (areLayerOrLevelDifferent(w1, w2)) {
    118116                return true;
    119             }
     117            if (areLayerOrLevelDifferent(w1, w2))
     118                return true;
    120119            if (isBuilding(w1) && isBuilding(w2))
    121                 return true;
    122             if (w1.hasKey(HIGHWAY) && w2.hasKey(HIGHWAY) && !Objects.equals(w1.get("level"), w2.get("level"))) {
    123                 return true;
    124             }
     120                return true; // handled by mapcss tests
    125121            if (((isResidentialArea(w1) || w1.hasKey(BARRIER, HIGHWAY, RAILWAY, WATERWAY)) && isResidentialArea(w2))
    126122             || ((isResidentialArea(w2) || w2.hasKey(BARRIER, HIGHWAY, RAILWAY, WATERWAY)) && isResidentialArea(w1)))
    127123                return true;
    128             if (isSubwayOrTramOrRazed(w2)) {
    129                 return true;
    130             }
     124            if (isWaterArea(w1) && isWaterArea(w2))
     125                return true; // handled by mapcss tests
    131126            if (w1.hasKey(RAILWAY) && w2.hasKey(RAILWAY) && w1.hasTag(RAILWAY, "yard") != w2.hasTag(RAILWAY, "yard")) {
    132127                return true; // see #20089
    133128            }
    134             if (isCoastline(w1) != isCoastline(w2)) {
    135                 return true;
    136             }
    137             if ((w1.hasTag(WATERWAY, "river", "stream", "canal", "drain", "ditch") && w2.hasTag(WATERWAY, "riverbank"))
    138              || (w2.hasTag(WATERWAY, "river", "stream", "canal", "drain", "ditch") && w1.hasTag(WATERWAY, "riverbank"))) {
    139                 return true;
    140             }
    141             return isProposedOrAbandoned(w2);
     129            return (w1.hasTag(WATERWAY, "river", "stream", "canal", "drain", "ditch") && w2.hasTag(WATERWAY, "riverbank"))
     130                    || (w2.hasTag(WATERWAY, "river", "stream", "canal", "drain", "ditch") && w1.hasTag(WATERWAY, "riverbank"));
    142131        }
    143132
     
    274263
    275264    /**
    276      * Self crossing ways test (for all the rest)
     265     * Self crossing ways test (for all ways)
    277266     */
    278267    public static class SelfCrossing extends CrossingWays {
    279268
    280269        protected static final int CROSSING_SELF = 604;
    281 
    282         CrossingWays.Ways normalTest = new Ways();
    283         CrossingWays.Boundaries boundariesTest = new Boundaries();
    284270
    285271        /**
     
    291277
    292278        @Override
    293         public boolean isPrimitiveUsable(OsmPrimitive p) {
    294             return super.isPrimitiveUsable(p) && !(normalTest.isPrimitiveUsable(p)
    295                     || boundariesTest.isPrimitiveUsable(p));
    296         }
    297 
    298         @Override
    299279        boolean ignoreWaySegmentCombination(Way w1, Way w2) {
    300             return w1 != w2; // should not happen
     280            return false; // we should not get here
    301281        }
    302282    }
     
    332312    }
    333313
     314    static boolean isWaterArea(OsmPrimitive w) {
     315        return w.hasTag("natural", "water") || w.hasTag(LANDUSE, "reservoir");
     316    }
     317
    334318    static boolean isHighway(OsmPrimitive w) {
    335319        return w.hasTagDifferent(HIGHWAY, "rest_area", "services", "bus_stop", "platform");
     
    358342    @Override
    359343    public void visit(Way w) {
    360         if (this instanceof SelfCrossing) {
     344        boolean findSelfCrossingOnly = this instanceof SelfCrossing;
     345        if (findSelfCrossingOnly) {
    361346            // free memory, we are not interested in previous ways
    362347            cellSegments.clear();
     
    378363                    List<WaySegment> highlight;
    379364
    380                     if (!es1.intersects(es2) || ignoreWaySegmentCombination(es1.way, es2.way)) {
     365                    if (!es1.intersects(es2)
     366                            || (!findSelfCrossingOnly && ignoreWaySegmentCombination(es1.way, es2.way))) {
    381367                        continue;
    382368                    }
     
    475461     * @return {@code true} if one or more segments of the way are crossing
    476462     * @see SelfIntersectingWay
    477      * @since xxx
     463     * @since 17393
    478464     */
    479465    public static boolean isSelfCrossing(Way way) {
  • trunk/test/unit/org/openstreetmap/josm/data/validation/tests/CrossingWaysTest.java

    r17275 r17400  
    66import static org.junit.jupiter.api.Assertions.assertTrue;
    77
     8import java.nio.file.Files;
     9import java.nio.file.Paths;
    810import java.util.HashMap;
    911import java.util.List;
    1012
     13import org.junit.jupiter.api.Test;
    1114import org.junit.jupiter.api.extension.RegisterExtension;
    12 import org.junit.jupiter.api.Test;
    1315import org.openstreetmap.josm.TestUtils;
    1416import org.openstreetmap.josm.data.coor.EastNorth;
    1517import org.openstreetmap.josm.data.coor.LatLon;
     18import org.openstreetmap.josm.data.osm.DataSet;
    1619import org.openstreetmap.josm.data.osm.Node;
    1720import org.openstreetmap.josm.data.osm.Way;
    1821import org.openstreetmap.josm.data.osm.WaySegment;
     22import org.openstreetmap.josm.data.validation.TestError;
    1923import org.openstreetmap.josm.data.validation.tests.CrossingWays.Boundaries;
    2024import org.openstreetmap.josm.data.validation.tests.CrossingWays.SelfCrossing;
    2125import org.openstreetmap.josm.data.validation.tests.CrossingWays.Ways;
     26import org.openstreetmap.josm.io.OsmReader;
    2227import org.openstreetmap.josm.testutils.JOSMTestRules;
    2328
     
    3439    @RegisterExtension
    3540    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
    36     public JOSMTestRules rule = new JOSMTestRules().preferences();
     41    public JOSMTestRules rule = new JOSMTestRules().preferences().projection();
    3742
    3843    private static Way newUsableWay(String tags) {
     
    127132        testMessage(601, test, "amenity=restaurant", "amenity=restaurant");
    128133        testMessage(611, test, "building=yes", "amenity=restaurant");
     134        testMessage(611, test, "building=yes", "natural=water");
    129135        testMessage(612, test, "building=yes", "highway=road");
    130136        testMessage(613, test, "building=yes", "railway=rail");
     
    133139        testMessage(620, test, "highway=road", "highway=road");
    134140        testMessage(621, test, "highway=road", "amenity=restaurant");
     141        testMessage(621, test, "highway=road", "natural=water");
    135142        testMessage(622, test, "highway=road", "railway=rail");
    136143        testMessage(623, test, "highway=road", "waterway=river");
    137144        testMessage(630, test, "railway=rail", "railway=rail");
    138145        testMessage(631, test, "railway=rail", "amenity=restaurant");
     146        testMessage(631, test, "railway=rail", "natural=water");
    139147        testMessage(632, test, "railway=rail", "waterway=river");
    140148        testMessage(641, test, "landuse=residential", "amenity=restaurant");
     
    146154        testMessage(663, test, "barrier=hedge", "railway=rail");
    147155        testMessage(664, test, "barrier=hedge", "waterway=river");
     156        testMessage(665, test, "barrier=hedge", "natural=water");
    148157
    149158        assertFalse(test.isPrimitiveUsable(newUsableWay("amenity=restaurant")));
     
    174183        SelfCrossing test = new CrossingWays.SelfCrossing();
    175184        // isPrimitiveUsable
    176         assertFalse(test.isPrimitiveUsable(newUsableWay("highway=motorway")));
    177         assertFalse(test.isPrimitiveUsable(newUsableWay("barrier=yes")));
    178         assertFalse(test.isPrimitiveUsable(newUsableWay("boundary=administrative")));
    179185        assertFalse(test.isPrimitiveUsable(TestUtils.newWay("amenity=restaurant"))); // Unusable (0 node)
    180         assertTrue(test.isPrimitiveUsable(newUsableWay("amenity=restaurant"))); // Usable (2 nodes)
    181     }
     186    }
     187
     188    /**
     189     * Various cases of crossing linear ways and areas, mainly for coverage
     190     * @throws Exception if an error occurs
     191     */
     192    @Test
     193    void testCoverage() throws Exception {
     194        DataSet ds = OsmReader.parseDataSet(
     195                Files.newInputStream(Paths.get(TestUtils.getTestDataRoot(), "crossingWays.osm")), null);
     196        CrossingWays crossingWays = new CrossingWays.Ways();
     197        crossingWays.startTest(null);
     198        crossingWays.visit(ds.allPrimitives());
     199        crossingWays.endTest();
     200
     201        for (TestError e : crossingWays.getErrors()) {
     202            // we don't report self crossing ways in this test
     203            assertEquals(2, e.getPrimitives().size(), e.getPrimitives().toString());
     204            // see #20121: crossing water areas should not be reported
     205            assertFalse(e.getPrimitives().stream().filter(Way.class::isInstance).allMatch(CrossingWays::isWaterArea));
     206        }
     207
     208        CrossingWays selfCrossing = new CrossingWays.SelfCrossing();
     209        selfCrossing.startTest(null);
     210        selfCrossing.visit(ds.allPrimitives());
     211        selfCrossing.endTest();
     212        assertEquals(1, selfCrossing.getErrors().size());
     213
     214        CrossingWays crossingBoundaries = new CrossingWays.Boundaries();
     215        crossingBoundaries.startTest(null);
     216        crossingBoundaries.visit(ds.allPrimitives());
     217        crossingBoundaries.endTest();
     218        assertEquals(2, crossingBoundaries.getErrors().size());
     219    }
     220
    182221}
Note: See TracChangeset for help on using the changeset viewer.