Changeset 15183 in josm for trunk

Timestamp:

2019-06-18T11:52:25+02:00 (6 years ago)

Author:

GerdP

Message:

fix #17768: Create / Update multipolygon did not work wich touching inner rings

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java (modified) (5 diffs)
• test/data/regress/17768/touching-inner-outer.osm (added)
• test/data/regress/17768/touching-inner.osm (added)
• test/unit/org/openstreetmap/josm/data/validation/tests/MultipolygonTestTest.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java

@@ -81 +81 @@
     /** set when used to build a multipolygon relation */
     private Relation createdRelation;
+    /** might be set when creating a relation and touching rings were found. */
+    private boolean repeatCheck;
 
     /**
@@ -250 +252 @@
             // we found no intersection or crossing between the polygons and they are closed
             // now we can calculate the nesting level to verify the roles with some simple node checks
-            checkRoles(r, allPolygons, wayMap, sharedNodes);
+            checkOrSetRoles(r, allPolygons, wayMap, sharedNodes);
         }
     }
@@ -461 +463 @@
      * @param sharedNodes all nodes shared by multiple ways of this multipolygon
      */
-    private void checkRoles(Relation r, List<PolyData> allPolygons, Map<Long, RelationMember> wayMap, Set<Node> sharedNodes) {
+    private void checkOrSetRoles(Relation r, List<PolyData> allPolygons, Map<Long, RelationMember> wayMap, Set<Node> sharedNodes) {
         PolygonLevelFinder levelFinder = new PolygonLevelFinder(sharedNodes);
         List<PolygonLevel> list = levelFinder.findOuterWays(allPolygons);
@@ -577 +579 @@
                         }
                     }
-                    if (loop == 0 || samePoly || (loop == 1 && !allInner)) {
+                    if (r == createdRelation && loop == 1 && !allInner) {
+                        repeatCheck = true;
+                    } else if (loop == 0 || samePoly || (loop == 1 && !allInner)) {
                         String msg = loop == 0 ? tr("Intersection between multipolygon ways")
                                 : samePoly ? tr("Multipolygon ring contains segments twice")
@@ -922 +926 @@
             r.addMember(new RelationMember("", w));
         }
-        errors.clear();
-        Multipolygon polygon = null;
-        boolean hasRepeatedMembers = checkRepeatedWayMembers(r);
-        if (!hasRepeatedMembers) {
-            polygon = new Multipolygon(r);
-            // don't check style consistency here
-            checkGeometryAndRoles(r, polygon);
-        }
-        createdRelation = null;
+        do {
+            repeatCheck = false;
+            errors.clear();
+            Multipolygon polygon = null;
+            boolean hasRepeatedMembers = checkRepeatedWayMembers(r);
+            if (!hasRepeatedMembers) {
+                polygon = new Multipolygon(r);
+                // don't check style consistency here
+                checkGeometryAndRoles(r, polygon);
+            }
+            createdRelation = null; // makes sure that repeatCheck is only set once
+        } while (repeatCheck);
         errors.removeIf(e->e.getSeverity() == Severity.OTHER);
         return r;

trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MultipolygonTestTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.data.validation.tests;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.InputStream;
 import java.util.stream.Collectors;
 
 import org.junit.Rule;
 import org.junit.Test;
+import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.osm.Relation;
+import org.openstreetmap.josm.io.OsmReader;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 
@@ -16 +22 @@
 public class MultipolygonTestTest {
 
-    private static final MultipolygonTest MULTIPOLYGON_TEST = new MultipolygonTest();
-    private static final RelationChecker RELATION_TEST = new RelationChecker();
 
     /**
@@ -24 +28 @@
     @Rule
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().projection().mapStyles().presets().main();
+    public JOSMTestRules test = new JOSMTestRules().projection().mapStyles().presets().main().preferences();
 
     /**
@@ -32 +36 @@
     @Test
     public void testMultipolygonFile() throws Exception {
+        final MultipolygonTest MULTIPOLYGON_TEST = new MultipolygonTest();
+        final RelationChecker RELATION_TEST = new RelationChecker();
         ValidatorTestUtils.testSampleFile("data_nodist/multipolygon.osm",
                 ds -> ds.getRelations().stream().filter(Relation::isMultipolygon).collect(Collectors.toList()),
                 name -> name.startsWith("06") || name.startsWith("07") || name.startsWith("08"), MULTIPOLYGON_TEST, RELATION_TEST);
     }
+
+    /**
+     * Non-regression test for ticket #17768.
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testTicket17768TouchingInner() throws Exception {
+        try (InputStream is = TestUtils.getRegressionDataStream(17768, "touching-inner.osm")) {
+            MultipolygonTest mpTest = new MultipolygonTest();
+            mpTest.makeFromWays(OsmReader.parseDataSet(is, null).getWays());
+            // inner touches inner, is considered OK in OSM
+            assertTrue(mpTest.getErrors().isEmpty());
+        }
+    }
+
+    /**
+     * Non-regression test for ticket #17768.
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testTicket17768TouchingInnerOuter() throws Exception {
+        try (InputStream is = TestUtils.getRegressionDataStream(17768, "touching-inner-outer.osm")) {
+            MultipolygonTest mpTest = new MultipolygonTest();
+            mpTest.makeFromWays(OsmReader.parseDataSet(is, null).getWays());
+            // inner touches outer, should return error
+            assertEquals(1, mpTest.getErrors().size());
+        }
+    }
 }