Ticket #19312: 19312.patch

src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java

@@ -8 +8 @@
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
@@ -14 +15 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
+import org.openstreetmap.josm.command.ChangeCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -61 +63 @@
     public static final int RELATION_EMPTY   = 1708;
     /** Type ''{0}'' of relation member with role ''{1}'' does not match accepted types ''{2}'' in preset {3} */
     public static final int WRONG_TYPE       = 1709;
+    /** Relations build a loop */
+    public static final int RELATION_LOOP    = 1710;
     // CHECKSTYLE.ON: SingleSpaceSeparator
 
     /**
@@ -70 +74 @@
     public static final String ROLE_VERIF_PROBLEM_MSG = tr("Role verification problem");
     private boolean ignoreMultiPolygons;
     private boolean ignoreTurnRestrictions;
-
+    private final List<List<Relation>> loops = new ArrayList<>();
     /**
      * Constructor
      */
@@ -154 +158 @@
         if (!map.isEmpty() && !allroles.isEmpty()) {
             checkRoles(n, allroles, map);
         }
+        checkLoop(n);
     }
 
     private static Map<String, RoleInfo> buildRoleInfoMap(Relation n) {
@@ -365 +370 @@
     public Command fixError(TestError testError) {
         Collection<? extends OsmPrimitive> primitives = testError.getPrimitives();
         if (isFixable(testError) && !primitives.iterator().next().isDeleted()) {
-            return new DeleteCommand(primitives);
+            if (testError.getCode() == RELATION_EMPTY) {
+                return new DeleteCommand(primitives);
+            }
+            if (testError.getCode() == RELATION_LOOP) {
+                Relation old = (Relation) primitives.iterator().next();
+                Relation mod = new Relation(old);
+                mod.removeMembersFor(primitives);
+                return new ChangeCommand(old, mod);
+            }
         }
         return null;
     }
@@ -373 +386 @@
     @Override
     public boolean isFixable(TestError testError) {
         Collection<? extends OsmPrimitive> primitives = testError.getPrimitives();
-        return testError.getCode() == RELATION_EMPTY && !primitives.isEmpty() && primitives.iterator().next().isNew();
+        return testError.getCode() == RELATION_EMPTY && !primitives.isEmpty() && primitives.iterator().next().isNew() ||
+                testError.getCode() == RELATION_LOOP && primitives.size() == 1;
     }
 
     @Override
@@ -381 +395 @@
         relationpresets.clear();
         initializePresets();
     }
+
+    @Override
+    public void endTest() {
+        loops.forEach(loop -> errors.add(TestError.builder(this, Severity.ERROR, RELATION_LOOP)
+                .message(loop.size() == 1 ? tr("Relation contains itself as a member") : tr("Relations build a dependency loop"))
+                .primitives(loop)
+                .build()));
+        loops.clear();
+        super.endTest();
+    }
+
+    private void checkLoop(Relation r) {
+        checkLoop(r, new LinkedList<>());
+    }
+
+    private void checkLoop(Relation parent, List<Relation> path) {
+        if (path.contains(parent)) {
+            Iterator<List<Relation>> iter = loops.iterator();
+            while (iter.hasNext()) {
+                List<Relation> loop = iter.next();
+                if (loop.size() > path.size() && loop.containsAll(path)) {
+                    // remove same loop with irrelevant parent
+                    iter.remove();
+                } else if (path.size() >= loop.size() && path.containsAll(loop)) {
+                    // same or smaller loop is already known
+                    return;
+                }
+            }
+            loops.add(path);
+            return;
+        }
+        path.add(parent);
+        for (Relation sub : parent.getMemberPrimitives(Relation.class)) {
+            if (sub.isUsable() && !sub.isIncomplete()) {
+                checkLoop(sub, new LinkedList<>(path));
+            }
+        }
+    }
+
 }