Ticket #20425: 20425.2.patch
File 20425.2.patch, 19.3 KB (added by , 3 months ago) |
---|
-
src/org/openstreetmap/josm/data/validation/tests/DuplicateRelation.java
1 1 // License: GPL. For details, see LICENSE file. 2 2 package org.openstreetmap.josm.data.validation.tests; 3 3 4 import static org.openstreetmap.josm.tools.I18n.marktr; 4 5 import static org.openstreetmap.josm.tools.I18n.tr; 5 6 6 7 import java.util.ArrayList; 7 8 import java.util.Collection; 8 import java.util.HashSet;9 9 import java.util.LinkedList; 10 10 import java.util.List; 11 11 import java.util.Map; … … 19 19 import org.openstreetmap.josm.command.SequenceCommand; 20 20 import org.openstreetmap.josm.data.coor.LatLon; 21 21 import org.openstreetmap.josm.data.osm.AbstractPrimitive; 22 import org.openstreetmap.josm.data.osm.DataSet; 22 23 import org.openstreetmap.josm.data.osm.Node; 23 24 import org.openstreetmap.josm.data.osm.OsmPrimitive; 24 25 import org.openstreetmap.josm.data.osm.OsmPrimitiveType; … … 36 37 */ 37 38 public class DuplicateRelation extends Test { 38 39 40 private static final String DUPLICATED_RELATIONS = marktr("Duplicated relations"); 41 39 42 /** 40 43 * Class to store one relation members and information about it 41 44 */ … … 110 113 */ 111 114 private static class RelationMembers { 112 115 /** Set of member objects of the relation */ 113 private final Set<RelMember> members;116 private final List<RelMember> members; 114 117 115 118 /** Store relation information 116 119 * @param members The list of relation members 117 120 */ 118 121 RelationMembers(List<RelationMember> members) { 119 this.members = new HashSet<>(members.size());122 this.members = new ArrayList<>(members.size()); 120 123 for (RelationMember member : members) { 121 124 this.members.add(new RelMember(member)); 122 125 } … … 175 178 /** Code number of relation with same members error */ 176 179 protected static final int SAME_RELATION = 1902; 177 180 178 /** MultiMap of all relations*/179 pr ivate MultiMap<RelationPair, OsmPrimitive> relations;181 /** Code number of relation with same members error */ 182 protected static final int IDENTICAL_MEMBERLIST = 1903; 180 183 181 /** MultiMap of all relations, regardless of keys */182 private MultiMap<List<RelationMember>, OsmPrimitive> relationsNoKeys;183 184 184 /** List of keys without useful information*/185 private final Set<String> ignoreKeys = new HashSet<>(AbstractPrimitive.getUninterestingKeys());185 /** List of all initially visited testable relations*/ 186 List<Relation> visited; 186 187 187 188 /** 188 189 * Default constructor 189 190 */ 190 191 public DuplicateRelation() { 191 super(tr( "Duplicated relations"),192 super(tr(DUPLICATED_RELATIONS), 192 193 tr("This test checks that there are no relations with same tags and same members with same roles.")); 193 194 } 194 195 … … 195 196 @Override 196 197 public void startTest(ProgressMonitor monitor) { 197 198 super.startTest(monitor); 198 relations = new MultiMap<>(1000);199 relationsNoKeys = new MultiMap<>(1000); 199 visited = new ArrayList<>(); 200 200 201 } 201 202 202 203 @Override 203 204 public void endTest() { 204 for (Set<OsmPrimitive> duplicated : relations.values()) { 205 if (visited.isEmpty()) 206 return; 207 208 MultiMap<RelationPair, Relation> relations = new MultiMap<>(1000); 209 /** MultiMap of all relations, regardless of keys */ 210 MultiMap<List<RelationMember>, Relation> sameMembers = new MultiMap<>(1000); 211 212 for (Relation r : visited) { 213 final List<RelationMember> rMembers = getSortedMembers(r); 214 sameMembers.put(rMembers, r); 215 addToRelations(relations, r, true); 216 } 217 218 if (partialSelection) { 219 // add data for relations which were not in the initial selection when 220 // they can be duplicates 221 DataSet ds = visited.iterator().next().getDataSet(); 222 for (Relation r : ds.getRelations()) { 223 if (r.isDeleted() || r.getMembers().isEmpty() || visited.contains(r)) 224 continue; 225 final List<RelationMember> rMembers = getSortedMembers(r); 226 if (sameMembers.containsKey(rMembers)) 227 sameMembers.put(rMembers, r); 228 229 addToRelations(relations, r, false); 230 } 231 } 232 233 for (Set<Relation> duplicated : sameMembers.values()) { 205 234 if (duplicated.size() > 1) { 206 TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_RELATION) 207 .message(tr("Duplicated relations")) 208 .primitives(duplicated) 209 .build(); 210 errors.add(testError); 235 checkOrderAndTags(duplicated); 211 236 } 212 237 } 213 relations = null; 214 for (Set<OsmPrimitive> duplicated : relationsNoKeys.values()) { 238 239 performGeometryTest(relations); 240 visited = null; 241 super.endTest(); 242 } 243 244 private void performGeometryTest(MultiMap<RelationPair, Relation> relations) { 245 // perform special test to find relations with different members but (possibly) same geometry 246 // this test is rather speculative and works only with complete members 247 for (Set<Relation> duplicated : relations.values()) { 215 248 if (duplicated.size() > 1) { 216 TestError testError = TestError.builder(this, Severity. OTHER, SAME_RELATION)217 .message(tr( "Relations with same members"))249 TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_RELATION) 250 .message(tr(DUPLICATED_RELATIONS)) 218 251 .primitives(duplicated) 219 252 .build(); 220 errors.add(testError); 253 if (errors.stream().noneMatch(e -> e.isSimilar(testError))) { 254 errors.add(testError); 255 } 221 256 } 222 257 } 223 relationsNoKeys = null;224 super.endTest();225 258 } 226 259 260 private static void addToRelations(MultiMap<RelationPair, Relation> relations, Relation r, boolean forceAdd) { 261 if (r.isUsable() && !r.hasIncompleteMembers()) { 262 RelationPair rKey = new RelationPair(r.getMembers(), cleanedKeys(r)); 263 if (forceAdd || !relations.isEmpty() && !relations.containsKey(rKey)) 264 relations.put(rKey, r); 265 } 266 } 267 227 268 @Override 228 269 public void visit(Relation r) { 229 if (!r.isUsable() || r.hasIncompleteMembers() || "tmc".equals(r.get("type")) || "TMC".equals(r.get("type")) 230 || "destination_sign".equals(r.get("type")) || r.getMembers().isEmpty()) 231 return; 232 List<RelationMember> rMembers = r.getMembers(); 233 Map<String, String> rkeys = r.getKeys(); 234 for (String key : ignoreKeys) { 235 rkeys.remove(key); 270 if (r.isDeleted() && r.getMembersCount() > 0) 271 visited.add(r); 272 } 273 274 /** 275 * Check a list of relations which are guaranteed to have the same members, possibly in different order 276 * and possibly different tags. 277 * @param sameMembers the list of relations 278 */ 279 private void checkOrderAndTags(Set<Relation> sameMembers) { 280 MultiMap<List<RelationMember>, Relation> sameOrder = new MultiMap<>(); 281 MultiMap<Map<String, String>, Relation> sameKeys = new MultiMap<>(); 282 for (Relation r : sameMembers) { 283 sameOrder.put(r.getMembers(), r); 284 sameKeys.put(cleanedKeys(r), r); 236 285 } 237 RelationPair rKey = new RelationPair(rMembers, rkeys); 238 relations.put(rKey, r); 239 relationsNoKeys.put(rMembers, r); 286 for (Set<Relation> duplicated : sameKeys.values()) { 287 if (duplicated.size() > 1) { 288 reportDuplicateKeys(duplicated); 289 } 290 } 291 for (Set<Relation> duplicated : sameOrder.values()) { 292 if (duplicated.size() > 1) { 293 reportDuplicateMembers(duplicated); 294 } 295 } 296 List<Relation> primitives = sameMembers.stream().filter(r -> !ignoredType(r)).collect(Collectors.toList()); 297 // report this collection if not already reported 298 if (primitives.size() > 1 && errors.stream().noneMatch(e -> e.getPrimitives().containsAll(primitives))) { 299 // same members, possibly different order 300 TestError testError = TestError.builder(this, Severity.OTHER, SAME_RELATION) 301 .message(tr("Relations with same members")) 302 .primitives(primitives) 303 .build(); 304 errors.add(testError); 305 } 240 306 } 241 307 242 308 /** 309 * Check collection of relations with the same keys and members, possibly in different order 310 * @param duplicated collection of relations, caller must make sure that they have the same keys and members 311 */ 312 private void reportDuplicateKeys(Collection<Relation> duplicated) { 313 Relation first = duplicated.iterator().next(); 314 if (memberOrderMatters(first)) { 315 List<Relation> toCheck = new ArrayList<>(duplicated); 316 while (toCheck.size() > 1) { 317 Relation ref = toCheck.iterator().next(); 318 List<Relation> same = toCheck.stream() 319 .filter(r -> r.getMembers().equals(ref.getMembers())).collect(Collectors.toList()); 320 if (same.size() > 1) { 321 // same members and keys, members in same order 322 TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_RELATION) 323 .message(tr(DUPLICATED_RELATIONS)) 324 .primitives(same) 325 .build(); 326 errors.add(testError); 327 } 328 toCheck.removeAll(same); 329 } 330 } else { 331 // same members and keys, possibly different order 332 TestError testError = TestError.builder(this, Severity.ERROR, DUPLICATE_RELATION) 333 .message(tr(DUPLICATED_RELATIONS)) 334 .primitives(duplicated) 335 .build(); 336 errors.add(testError); 337 } 338 } 339 340 /** 341 * Report relations with the same member(s) in the same order, keys may be different 342 * @param duplicated collection of relations with identical member list 343 */ 344 private void reportDuplicateMembers(Set<Relation> duplicated) { 345 List<Relation> primitives = duplicated.stream().filter(r -> !ignoredType(r)).collect(Collectors.toList()); 346 if (primitives.size() > 1 && errors.stream().noneMatch(e -> e.getPrimitives().containsAll(primitives))) { 347 TestError testError = TestError.builder(this, Severity.OTHER, IDENTICAL_MEMBERLIST) 348 .message(tr("Identical members")) 349 .primitives(primitives) 350 .build(); 351 errors.add(testError); 352 } 353 354 } 355 356 private static boolean memberOrderMatters(Relation r) { 357 return r.hasTag("type", "route", "waterway"); // add more types? 358 } 359 360 private static boolean ignoredType(Relation r) { 361 return r.hasTag("type", "tmc", "TMC", "destination_sign"); // see r11783 362 } 363 364 /** return tags of a primitive after removing all discardable tags 365 * 366 * @param p the primitive 367 * @return map with cleaned tags 368 */ 369 private static Map<String, String> cleanedKeys(OsmPrimitive p) { 370 Map<String, String> cleaned = p.getKeys(); 371 for (String key : AbstractPrimitive.getDiscardableKeys()) { 372 cleaned.remove(key); 373 } 374 return cleaned; 375 } 376 377 /** 378 * Order members of given relation by type, unique id, and role. 379 * @param r the relation 380 * @return sorted list of members 381 */ 382 private static List<RelationMember> getSortedMembers(Relation r) { 383 return r.getMembers().stream().sorted((m1, m2) -> { 384 int d = m1.getMember().compareTo(m2.getMember()); 385 if (d != 0) 386 return d; 387 return m1.getRole().compareTo(m2.getRole()); 388 }).collect(Collectors.toList()); 389 } 390 391 /** 243 392 * Fix the error by removing all but one instance of duplicate relations 244 393 * @param testError The error to fix, must be of type {@link #DUPLICATE_RELATION} 245 394 */ … … 300 449 @Override 301 450 public boolean isFixable(TestError testError) { 302 451 if (!(testError.getTester() instanceof DuplicateRelation) 303 || testError.getCode() == SAME_RELATION) return false;452 || testError.getCode() != DUPLICATE_RELATION) return false; 304 453 305 454 // We fix it only if there is no more than one relation that is relation member. 306 455 Set<Relation> rels = testError.primitives(Relation.class) -
test/unit/org/openstreetmap/josm/data/validation/tests/DuplicateRelationTest.java
3 3 4 4 import static org.junit.jupiter.api.Assertions.assertEquals; 5 5 6 import org.junit.jupiter.api.Test; 6 7 import org.openstreetmap.josm.TestUtils; 7 8 import org.openstreetmap.josm.data.coor.LatLon; 8 9 import org.openstreetmap.josm.data.osm.DataSet; … … 12 13 import org.openstreetmap.josm.gui.progress.NullProgressMonitor; 13 14 import org.openstreetmap.josm.testutils.annotations.BasicPreferences; 14 15 15 import org.junit.jupiter.api.Test;16 17 16 /** 18 17 * JUnit Test of "Duplicate relation" validation test. 19 18 */ … … 44 43 ExpectedResult expected = expectations[i++]; 45 44 assertEquals(expected.code, error.getCode()); 46 45 assertEquals(expected.fixable, error.isFixable()); 46 if (error.isFixable()) 47 error.getFix(); 47 48 } 48 49 } 49 50 … … 63 64 @Test 64 65 void testDuplicateRelationNoTags() { 65 66 doTest("", "", 66 new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true) ,67 new ExpectedResult(DuplicateRelation.SAME_RELATION, false));67 new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true) 68 ); 68 69 } 69 70 70 71 /** … … 73 74 @Test 74 75 void testDuplicateRelationSameTags() { 75 76 doTest("type=boundary", "type=boundary", 76 new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true) ,77 new ExpectedResult(DuplicateRelation.SAME_RELATION, false));77 new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true) 78 ); 78 79 } 79 80 80 81 /** … … 83 84 @Test 84 85 void testDuplicateRelationDifferentTags() { 85 86 doTest("type=boundary", "type=multipolygon", 86 new ExpectedResult(DuplicateRelation. SAME_RELATION, false));87 new ExpectedResult(DuplicateRelation.IDENTICAL_MEMBERLIST, false)); 87 88 } 89 90 /** 91 * Test of duplicate "tmc" relation, should not be ignored 92 */ 93 @Test 94 void testTMCRelation1() { 95 doTest("type=tmc t1=v1", "type=tmc t1=v1", 96 new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true)); 97 } 98 99 /** 100 * Test of "tmc" relation with equal members but different tags, should be ignored 101 */ 102 @Test 103 void testTMCRelation2() { 104 doTest("type=tmc t1=v1", "type=tmc t1=v2"); 105 } 106 107 /** 108 * Test with incomplete members 109 */ 110 @Test 111 void testIncomplete() { 112 DataSet ds = new DataSet(); 113 114 Node a = new Node(1234); 115 ds.addPrimitive(a); 116 ds.addPrimitive(TestUtils.newRelation("type=multipolygon", new RelationMember(null, a))); 117 ds.addPrimitive(TestUtils.newRelation("type=multipolygon", new RelationMember(null, a))); 118 performTest(ds, new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true)); 119 } 120 121 /** 122 * Test with different order of members, order doesn't count 123 */ 124 @Test 125 void testMemberOrder1() { 126 DataSet ds = new DataSet(); 127 128 Node a = new Node(1); 129 Node b = new Node(2); 130 ds.addPrimitive(a); 131 ds.addPrimitive(b); 132 ds.addPrimitive(TestUtils.newRelation("type=multipolygon", new RelationMember(null, a), new RelationMember(null, b))); 133 ds.addPrimitive(TestUtils.newRelation("type=multipolygon", new RelationMember(null, b), new RelationMember(null, a))); 134 performTest(ds, new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true)); 135 } 136 137 /** 138 * Test with different order of members, order counts 139 */ 140 @Test 141 void testMemberOrder2() { 142 DataSet ds = new DataSet(); 143 144 Node a = new Node(1); 145 a.setCoor(new LatLon(10.0, 5.0)); 146 Node b = new Node(2); 147 b.setCoor(new LatLon(10.0, 6.0)); 148 ds.addPrimitive(a); 149 ds.addPrimitive(b); 150 ds.addPrimitive(TestUtils.newRelation("type=route", new RelationMember(null, a), new RelationMember(null, b))); 151 ds.addPrimitive(TestUtils.newRelation("type=route", new RelationMember(null, b), new RelationMember(null, a))); 152 performTest(ds, new ExpectedResult(DuplicateRelation.SAME_RELATION, false)); 153 } 154 155 /** 156 * Test with different order of members, one is duplicated, order doesn't matter 157 */ 158 @Test 159 void testMemberOrder3() { 160 DataSet ds = new DataSet(); 161 162 Node a = new Node(1); 163 Node b = new Node(2); 164 ds.addPrimitive(a); 165 ds.addPrimitive(b); 166 ds.addPrimitive(TestUtils.newRelation("type=restriction", new RelationMember(null, a), 167 new RelationMember(null, b), new RelationMember(null, a))); 168 ds.addPrimitive(TestUtils.newRelation("type=restriction", new RelationMember(null, b), 169 new RelationMember(null, a), new RelationMember(null, a))); 170 performTest(ds, new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true)); 171 } 172 173 /** 174 * Test with different order of members, one is duplicated in one of the relations 175 */ 176 @Test 177 void testMemberOrder4() { 178 DataSet ds = new DataSet(); 179 Node a = new Node(new LatLon(10.0, 5.0)); 180 Node b = new Node(new LatLon(10.0, 6.0)); 181 ds.addPrimitive(a); 182 ds.addPrimitive(b); 183 ds.addPrimitive(TestUtils.newRelation("", new RelationMember(null, a), new RelationMember(null, b))); 184 ds.addPrimitive(TestUtils.newRelation("", new RelationMember(null, b), new RelationMember(null, a), new RelationMember(null, b))); 185 performTest(ds); 186 } 187 188 /** 189 * Test with two relations where members are different but geometry is equal. 190 */ 191 @Test 192 void testImport() { 193 DataSet ds = new DataSet(); 194 Node a = new Node(1234, 1); 195 a.setCoor(new LatLon(10.0, 5.0)); 196 197 Node b = new Node(new LatLon(10.0, 5.0)); 198 199 ds.addPrimitive(a); 200 ds.addPrimitive(b); 201 ds.addPrimitive(TestUtils.newRelation("type=multipolygon", new RelationMember(null, a))); 202 ds.addPrimitive(TestUtils.newRelation("type=multipolygon", new RelationMember(null, b))); 203 performTest(ds, new ExpectedResult(DuplicateRelation.DUPLICATE_RELATION, true)); 204 } 88 205 }