Changeset 4370 in josm

Timestamp:

Aug 28, 2011 1:26:45 PM (21 months ago)

Author:

stoecker

Message:

fix #5861 - patch by simon04 - validator warnings for multiple via ways

Location:

trunk

Files:

• data/defaultpresets.xml (modified) (1 diff)
• src/org/openstreetmap/josm/data/validation/tests/TurnrestrictionTest.java (modified) (8 diffs)

trunk/data/defaultpresets.xml

@@ -4302 +4302 @@
             <roles>
                 <role key="from" text="from way" requisite="required" count="1" type="way" />
-                <role key="via" text="via node or way" requisite="required" count="1" type="way,node" />
+                <role key="via" text="via node or ways" requisite="required" type="way,node" />
                 <role key="to" text="to way" requisite="required" count="1" type="way" />
             </roles>

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

@@ -5 +5 @@
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 
 import org.openstreetmap.josm.data.osm.Node;
@@ -31 +33 @@
     protected static final int FROM_VIA_WAY = 1811;
     protected static final int TO_VIA_WAY = 1812;
+    protected static final int MIX_VIA = 1813;
+    protected static final int UNCONNECTED_VIA = 1814;
 
     public TurnrestrictionTest() {
@@ -43 +47 @@
         Way fromWay = null;
         Way toWay = null;
-        OsmPrimitive via = null;
+        List<OsmPrimitive> via = new ArrayList<OsmPrimitive>();
 
         boolean morefrom = false;
         boolean moreto = false;
         boolean morevia = false;
+        boolean mixvia = false;
 
         /* find the "from", "via" and "to" elements */
@@ -76 +81 @@
                     }
                 } else if ("via".equals(m.getRole())) {
-                    if (via != null) {
-                        morevia = true;
-                    } else {
-                        via = w;
+                    if (!via.isEmpty() && via.get(0) instanceof Node) {
+                        mixvia = true;
+                    } else {
+                        via.add(w);
                     }
                 } else {
@@ -88 +93 @@
                 Node n = m.getNode();
                 if ("via".equals(m.getRole())) {
-                    if (via != null) {
-                        morevia = true;
-                    } else {
-                        via = n;
+                    if (!via.isEmpty()) {
+                        if (via.get(0) instanceof Node) {
+                            morevia = true;
+                        } else {
+                            mixvia = true;
+                        }
+                    } else {
+                        via.add(n);
                     }
                 } else {
@@ -109 +118 @@
         }
         if (morevia) {
-            errors.add(new TestError(this, Severity.ERROR, tr("More than one \"via\" way found"), MORE_VIA, r));
+            errors.add(new TestError(this, Severity.ERROR, tr("More than one \"via\" node found"), MORE_VIA, r));
+        }
+        if (mixvia) {
+            errors.add(new TestError(this, Severity.ERROR, tr("Cannot mix node and way for role \"via\""), MIX_VIA, r));
         }
 
@@ -120 +132 @@
             return;
         }
-        if (via == null) {
+        if (via.isEmpty()) {
             errors.add(new TestError(this, Severity.ERROR, tr("No \"via\" node or way found"), NO_VIA, r));
             return;
@@ -126 +138 @@
 
         Node viaNode;
-        if (via instanceof Node) {
-            viaNode = (Node) via;
-            if (!fromWay.isFirstLastNode(viaNode)) {
-                errors.add(new TestError(this, Severity.ERROR,
-                        tr("The \"from\" way does not start or end at a \"via\" node"), FROM_VIA_NODE, r));
-                return;
+        if (via.get(0) instanceof Node) {
+            viaNode = (Node) via.get(0);
+            Way viaPseudoWay = new Way();
+            viaPseudoWay.addNode(viaNode);
+            checkIfConnected(fromWay, viaPseudoWay,
+                    tr("The \"from\" way does not start or end at a \"via\" node"), FROM_VIA_NODE);
+            checkIfConnected(viaPseudoWay, toWay,
+                    tr("The \"to\" way does not start or end at a \"via\" node"), TO_VIA_NODE);
+        } else {
+            // check if consecutive ways are connected: from/via[0], via[i-1]/via[i], via[last]/to
+            checkIfConnected(fromWay, (Way) via.get(0), 
+                    tr("The \"from\" and the first \"via\" way are not connected."), FROM_VIA_WAY);
+            if (via.size() > 1) {
+                for (int i = 1; i < via.size(); i++) {
+                    Way previous = (Way) via.get(i - 1);
+                    Way current = (Way) via.get(i);
+                    checkIfConnected(previous, current, 
+                            tr("The \"via\" ways are not connected."), UNCONNECTED_VIA);
+                }
             }
-            if (!toWay.isFirstLastNode(viaNode)) {
-                errors.add(new TestError(this, Severity.ERROR,
-                        tr("The \"to\" way does not start or end at a \"via\" node"), TO_VIA_NODE, r));
-                return;
+            checkIfConnected((Way) via.get(via.size() - 1), toWay, 
+                    tr("The last \"via\" and the \"to\" way are not connected."), TO_VIA_WAY);
+
+        }
+    }
+
+    private void checkIfConnected(Way previous, Way current, String msg, int code) {
+        int onewayPrevious = isOneway(previous);
+        int onewayCurrent = isOneway(current);
+        Node endPrevious = onewayPrevious != -1 ? previous.lastNode() : previous.firstNode();
+        Node startCurrent = onewayCurrent != -1 ? current.firstNode() : current.lastNode();
+        //System.out.println(previous.getUniqueId() + " -- " + current.getUniqueId() + ": " + onewayPrevious + "/" + onewayCurrent + " " + endPrevious.getUniqueId() + "/" + startCurrent.getUniqueId());
+        boolean c;
+        if (onewayPrevious != 0 && onewayCurrent != 0) {
+            // both oneways: end/start node must be equal
+            c = endPrevious.equals(startCurrent);
+        } else if (onewayPrevious != 0) {
+            // previous way is oneway: end of previous must be start/end of current
+            c = current.isFirstLastNode(endPrevious);
+        } else if (onewayCurrent != 0) {
+            // current way is oneway: start of current must be start/end of previous
+            c = previous.isFirstLastNode(startCurrent);
+        } else {
+            // otherwise: start/end of previous must be start/end of current
+            c = current.isFirstLastNode(previous.firstNode()) || current.isFirstLastNode(previous.lastNode());
+        }
+        if (!c) {
+            errors.add(new TestError(this, Severity.ERROR, msg, code, Arrays.asList(previous, current)));
+        }
+    }
+
+    private static int isOneway(Way w) {
+        String onewayviastr = w.get("oneway");
+        if (onewayviastr != null) {
+            if ("-1".equals(onewayviastr)) {
+                return -1;
+            } else {
+                Boolean onewayvia = OsmUtils.getOsmBoolean(onewayviastr);
+                if (onewayvia != null && onewayvia) {
+                    return 1;
+                }
             }
-        } else {
-            Way viaWay = (Way) via;
-            Node firstNode = viaWay.firstNode();
-            Node lastNode = viaWay.lastNode();
-            Boolean onewayvia = false;
-
-            String onewayviastr = viaWay.get("oneway");
-            if (onewayviastr != null) {
-                if ("-1".equals(onewayviastr)) {
-                    onewayvia = true;
-                    Node tmp = firstNode;
-                    firstNode = lastNode;
-                    lastNode = tmp;
-                } else {
-                    onewayvia = OsmUtils.getOsmBoolean(onewayviastr);
-                    if (onewayvia == null) {
-                        onewayvia = false;
-                    }
-                }
-            }
-
-            if (fromWay.isFirstLastNode(firstNode)) {
-                viaNode = firstNode;
-            } else if (!onewayvia && fromWay.isFirstLastNode(lastNode)) {
-                viaNode = lastNode;
-            } else {
-                errors.add(new TestError(this, Severity.ERROR,
-                        tr("The \"from\" way does not start or end at a \"via\" way."), FROM_VIA_WAY, r));
-                return;
-            }
-            if (!toWay.isFirstLastNode(viaNode == firstNode ? lastNode : firstNode)) {
-                errors.add(new TestError(this, Severity.ERROR,
-                        tr("The \"to\" way does not start or end at a \"via\" way."), TO_VIA_WAY, r));
-            }
-        }
+        }
+        return 0;
     }
 }