Ticket #22614: 22614_check_eulerian.patch

src/org/openstreetmap/josm/actions/CombineWayAction.java

@@ -232 +232 @@
     }
 
     protected static List<Node> tryJoin(Collection<Way> ways) {
-        List<Node> path = joinWithMultipolygonCode(ways);
+        List<Node> path;
+        NodeGraph graph = NodeGraph.createDirectedGraphFromWays(ways);
+        path = graph.buildSpanningPathNoRemove();
         if (path.isEmpty()) {
-            NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ways);
+            path = joinWithMultipolygonCode(ways);
+        }
+        if (path.isEmpty()) {
+            graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ways);
             path = graph.buildSpanningPathNoRemove();
         }
         return path;

src/org/openstreetmap/josm/data/osm/NodeGraph.java

@@ -11 +11 @@
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -140 +141 @@
     private int numUndirectedEges;
     /** counts the number of edges that were added */
     private int addedEdges;
+    private LinkedList<Integer>[] adj;
+    private final List<Node> allNodes = new ArrayList<>();
+    private final HashMap<Node, Integer> nodeIdMap = new HashMap<>();
     private final Map<Node, List<NodePair>> successors = new LinkedHashMap<>();
     private final Map<Node, List<NodePair>> predecessors = new LinkedHashMap<>();
 
@@ -182 +186 @@
             rememberPredecessors(pair);
         }
         numUndirectedEges = undirectedEdges.size();
+
+        // calculate an index for each node contained in this graph
+        allNodes.clear();
+        allNodes.addAll(getNodes());
+        // calculate the adjen
+        nodeIdMap.clear();
+        adj = new LinkedList[allNodes.size()];
+        for (int i = 0; i < allNodes.size(); i++) {
+            adj[i] = new LinkedList<>();
+        }
+        // map the nodes in this graph to an Integer
+        for (int i = 0; i < allNodes.size(); i++) {
+            nodeIdMap.put(allNodes.get(i), i);
+        }
+
+        for (NodePair edge : undirectedEdges) {
+            int idxA = nodeIdMap.get(edge.getA());
+            int idxB = nodeIdMap.get(edge.getB());
+            addEdge(idxA, idxB);
+        }
     }
 
     /**
@@ -210 +234 @@
         }
     }
 
-    protected Set<Node> getTerminalNodes() {
-        return getNodes().stream().filter(this::isTerminalNode).collect(Collectors.toCollection(LinkedHashSet::new));
-    }
-
     private List<NodePair> getConnectedPairs(Node node) {
         List<NodePair> connected = new ArrayList<>();
         connected.addAll(Optional.ofNullable(successors.get(node)).orElseGet(Collections::emptyList));
@@ -287 +307 @@
      */
     public List<Node> buildSpanningPath() {
         prepare();
-        if (numUndirectedEges > 0 && isConnected()) {
-            // try to find a path from each "terminal node", i.e. from a
-            // node which is connected by exactly one undirected edges (or
-            // two directed edges in opposite direction) to the graph. A
-            // graph built up from way segments is likely to include such
-            // nodes, unless the edges build one or more closed rings.
-            // We order the nodes to start with the best candidates, but
-            // it might take very long if there is no valid path as we iterate over all nodes
-            // to find out.
-            Set<Node> nodes = getTerminalNodes();
-            nodes = nodes.isEmpty() ? getMostFrequentVisitedNodesFirst() : nodes;
-            return nodes.stream()
-                    .map(this::buildSpanningPath)
-                    .filter(path -> !path.isEmpty())
-                    .findFirst().orElse(null);
+        if (numUndirectedEges > 0) {
+            int indicator = isEulerian();
+            if (indicator > 0) {
+                // try to find a path from each "terminal node", i.e. from a
+                // node which is connected by exactly one undirected edges (or
+                // two directed edges in opposite direction) to the graph. A
+                // graph built up from way segments is likely to include such
+                // nodes, unless the edges build one or more closed rings.
+                // We order the nodes to start with the best candidates, but
+                // it might take very long if there is no valid path as we iterate over all nodes
+                // to find out.
+                Collection<Node> startNodes = getOddNodes();
+                startNodes = startNodes.isEmpty() ? getMostFrequentVisitedNodesFirst() : startNodes;
+                return startNodes.stream()
+                        .map(this::buildSpanningPath)
+                        .filter(path -> !path.isEmpty())
+                        .findFirst().orElse(null);
+            }
         }
         return null;
     }
@@ -327 +350 @@
      * @return true if it is connected.
      */
     private boolean isConnected() {
-        Set<Node> nodes = getNodes();
-        if (nodes.isEmpty())
+        if (allNodes.isEmpty())
             return false;
         Deque<Node> toVisit = new ArrayDeque<>();
         HashSet<Node> visited = new HashSet<>();
-        toVisit.add(nodes.iterator().next());
+        toVisit.add(allNodes.iterator().next());
         while (!toVisit.isEmpty()) {
             Node n = toVisit.pop();
             if (!visited.contains(n)) {
@@ -345 +367 @@
                 visited.add(n);
             }
         }
-        return nodes.size() == visited.size();
+        return allNodes.size() == visited.size();
     }
 
     /**
@@ -377 +399 @@
         return Collections.unmodifiableSet(result);
     }
 
+    private void addEdge(int v, int w) {
+        //Function to add an edge into the graph
+        adj[v].add(w); // Add w to v's list.
+        adj[w].add(v); //The graph is undirected
+    }
+
+    private List<Node> getOddNodes() {
+        List<Node> odd = new ArrayList<>();
+        // Count vertices with odd degree
+        for (int i = 0; i < allNodes.size(); i++) {
+            if (adj[i].size() % 2 != 0)
+                odd.add(allNodes.get(i));
+        }
+        return odd;
+    }
+
+    /**
+     * Check if graph has Eulerian path or Circuit or none of both
+     * Code taken from https://www.geeksforgeeks.org/eulerian-path-and-circuit/
+     * @return 0 if graph is not Eulerian, 1 if there is a Eulerian Path, 2 if there is an Eulerian Circuit
+     */
+    private int isEulerian() {
+        // Check if all vertices are connected
+        if (!isConnected())
+            return 0;
+
+        // Count vertices with odd degree
+        int odd = 0;
+        for (int i = 0; i < allNodes.size(); i++) {
+            if (adj[i].size() % 2 != 0)
+                odd++;
+        }
+
+        // If count is more than 2, then graph is not Eulerian
+        if (odd > 2)
+            return 0;
+
+        // If odd count is 2, then semi-eulerian.
+        // If odd count is 0, then eulerian
+        // Note that odd count can never be 1 for undirected graph
+        return (odd == 2) ? 1 : 2;
+    }
 }

src/org/openstreetmap/josm/data/osm/NodePair.java

@@ -77 +77 @@
     public String toString() {
         return new StringBuilder()
         .append('[')
-        .append(a.getId())
+        .append(a.getUniqueId())
         .append(',')
-        .append(b.getId())
+        .append(b.getUniqueId())
         .append(']')
         .toString();
     }

test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java

@@ -3 +3 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -14 +15 @@
 import java.util.List;
 import java.util.Set;
 
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import org.junit.jupiter.api.Test;
 import org.openstreetmap.josm.TestUtils;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Node;
@@ -137 +138 @@
             List<Way> reversedWays = new LinkedList<>();
             List<Way> unreversedWays = new LinkedList<>();
             CombineWayAction.detectReversedWays(selection, path, reversedWays, unreversedWays);
-            assertFalse(reversedWays.isEmpty());
+            assertTrue(reversedWays.isEmpty());
         }
     }