Changeset 11007 in josm


Ignore:
Timestamp:
2016-09-17T14:37:53+02:00 (8 years ago)
Author:
Don-vip
Message:

fix #13643 - prevent creation of DuplicateNode errors with empty list of primitives, improve unit test

Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java

    r10970 r11007  
    3232import org.openstreetmap.josm.data.validation.TestError;
    3333import org.openstreetmap.josm.gui.progress.ProgressMonitor;
     34import org.openstreetmap.josm.tools.CheckParameterUtil;
    3435import org.openstreetmap.josm.tools.MultiMap;
    3536
    … …  
    8687    }
    8788
     89    private static class DuplicateNodeTestError extends TestError {
     90        DuplicateNodeTestError(Test parentTest, Severity severity, String msg, int code, Set<OsmPrimitive> primitives) {
     91            super(parentTest, severity, tr("Duplicated nodes"), tr(msg), msg, code, primitives);
     92            CheckParameterUtil.ensureThat(!primitives.isEmpty(), "Empty primitives: " + msg);
     93        }
     94    }
     95
    8896    protected static final int DUPLICATE_NODE = 1;
    8997    protected static final int DUPLICATE_NODE_MIXED = 2;
    … …  
    173181        // check whether we have multiple nodes at the same position with the same tag set
    174182        for (Iterator<Map<String, String>> it = mm.keySet().iterator(); it.hasNext();) {
    175             Map<String, String> tagSet = it.next();
    176             if (mm.get(tagSet).size() > 1) {
     183            Set<OsmPrimitive> primitives = mm.get(it.next());
     184            if (primitives.size() > 1) {
    177185
    178186                for (String type: TYPES) {
    … …  
    180188                }
    181189
    182                 for (OsmPrimitive p : mm.get(tagSet)) {
     190                for (OsmPrimitive p : primitives) {
    183191                    if (p.getType() == OsmPrimitiveType.NODE) {
    184192                        Node n = (Node) p;
    … …  
    203211                }
    204212
    205                 int nbType = 0;
    206                 for (Entry<String, Boolean> entry: typeMap.entrySet()) {
    207                     if (entry.getValue()) {
    208                         nbType++;
    209                     }
    210                 }
     213                long nbType = typeMap.entrySet().stream().filter(Entry::getValue).count();
    211214
    212215                if (nbType > 1) {
    213                     String msg = marktr("Mixed type duplicated nodes");
    214                     errors.add(new TestError(
     216                    errors.add(new DuplicateNodeTestError(
    215217                            parentTest,
    216218                            Severity.WARNING,
    217                             tr("Duplicated nodes"),
    218                             tr(msg),
    219                             msg,
     219                            marktr("Mixed type duplicated nodes"),
    220220                            DUPLICATE_NODE_MIXED,
    221                             mm.get(tagSet)
     221                            primitives
    222222                            ));
    223223                } else if (typeMap.get("highway")) {
    224                     String msg = marktr("Highway duplicated nodes");
    225                     errors.add(new TestError(
    226                             parentTest,
    227                             Severity.ERROR,
    228                             tr("Duplicated nodes"),
    229                             tr(msg),
    230                             msg,
     224                    errors.add(new DuplicateNodeTestError(
     225                            parentTest,
     226                            Severity.ERROR,
     227                            marktr("Highway duplicated nodes"),
    231228                            DUPLICATE_NODE_HIGHWAY,
    232                             mm.get(tagSet)
     229                            primitives
    233230                            ));
    234231                } else if (typeMap.get("railway")) {
    235                     String msg = marktr("Railway duplicated nodes");
    236                     errors.add(new TestError(
    237                             parentTest,
    238                             Severity.ERROR,
    239                             tr("Duplicated nodes"),
    240                             tr(msg),
    241                             msg,
     232                    errors.add(new DuplicateNodeTestError(
     233                            parentTest,
     234                            Severity.ERROR,
     235                            marktr("Railway duplicated nodes"),
    242236                            DUPLICATE_NODE_RAILWAY,
    243                             mm.get(tagSet)
     237                            primitives
    244238                            ));
    245239                } else if (typeMap.get("waterway")) {
    246                     String msg = marktr("Waterway duplicated nodes");
    247                     errors.add(new TestError(
    248                             parentTest,
    249                             Severity.ERROR,
    250                             tr("Duplicated nodes"),
    251                             tr(msg),
    252                             msg,
     240                    errors.add(new DuplicateNodeTestError(
     241                            parentTest,
     242                            Severity.ERROR,
     243                            marktr("Waterway duplicated nodes"),
    253244                            DUPLICATE_NODE_WATERWAY,
    254                             mm.get(tagSet)
     245                            primitives
    255246                            ));
    256247                } else if (typeMap.get("boundary")) {
    257                     String msg = marktr("Boundary duplicated nodes");
    258                     errors.add(new TestError(
    259                             parentTest,
    260                             Severity.ERROR,
    261                             tr("Duplicated nodes"),
    262                             tr(msg),
    263                             msg,
     248                    errors.add(new DuplicateNodeTestError(
     249                            parentTest,
     250                            Severity.ERROR,
     251                            marktr("Boundary duplicated nodes"),
    264252                            DUPLICATE_NODE_BOUNDARY,
    265                             mm.get(tagSet)
     253                            primitives
    266254                            ));
    267255                } else if (typeMap.get("power")) {
    268                     String msg = marktr("Power duplicated nodes");
    269                     errors.add(new TestError(
    270                             parentTest,
    271                             Severity.ERROR,
    272                             tr("Duplicated nodes"),
    273                             tr(msg),
    274                             msg,
     256                    errors.add(new DuplicateNodeTestError(
     257                            parentTest,
     258                            Severity.ERROR,
     259                            marktr("Power duplicated nodes"),
    275260                            DUPLICATE_NODE_POWER,
    276                             mm.get(tagSet)
     261                            primitives
    277262                            ));
    278263                } else if (typeMap.get("natural")) {
    279                     String msg = marktr("Natural duplicated nodes");
    280                     errors.add(new TestError(
    281                             parentTest,
    282                             Severity.ERROR,
    283                             tr("Duplicated nodes"),
    284                             tr(msg),
    285                             msg,
     264                    errors.add(new DuplicateNodeTestError(
     265                            parentTest,
     266                            Severity.ERROR,
     267                            marktr("Natural duplicated nodes"),
    286268                            DUPLICATE_NODE_NATURAL,
    287                             mm.get(tagSet)
     269                            primitives
    288270                            ));
    289271                } else if (typeMap.get("building")) {
    290                     String msg = marktr("Building duplicated nodes");
    291                     errors.add(new TestError(
    292                             parentTest,
    293                             Severity.ERROR,
    294                             tr("Duplicated nodes"),
    295                             tr(msg),
    296                             msg,
     272                    errors.add(new DuplicateNodeTestError(
     273                            parentTest,
     274                            Severity.ERROR,
     275                            marktr("Building duplicated nodes"),
    297276                            DUPLICATE_NODE_BUILDING,
    298                             mm.get(tagSet)
     277                            primitives
    299278                            ));
    300279                } else if (typeMap.get("landuse")) {
    301                     String msg = marktr("Landuse duplicated nodes");
    302                     errors.add(new TestError(
    303                             parentTest,
    304                             Severity.ERROR,
    305                             tr("Duplicated nodes"),
    306                             tr(msg),
    307                             msg,
     280                    errors.add(new DuplicateNodeTestError(
     281                            parentTest,
     282                            Severity.ERROR,
     283                            marktr("Landuse duplicated nodes"),
    308284                            DUPLICATE_NODE_LANDUSE,
    309                             mm.get(tagSet)
     285                            primitives
    310286                            ));
    311287                } else {
    312                     String msg = marktr("Other duplicated nodes");
    313                     errors.add(new TestError(
     288                    errors.add(new DuplicateNodeTestError(
    314289                            parentTest,
    315290                            Severity.WARNING,
    316                             tr("Duplicated nodes"),
    317                             tr(msg),
    318                             msg,
     291                            marktr("Other duplicated nodes"),
    319292                            DUPLICATE_NODE_OTHER,
    320                             mm.get(tagSet)
    321                             ));
    322 
     293                            primitives
     294                            ));
    323295                }
    324296                it.remove();
    … …  
    326298        }
    327299
    328         // check whether we have multiple nodes at the same position with
    329         // differing tag sets
    330         //
     300        // check whether we have multiple nodes at the same position with differing tag sets
    331301        if (!mm.isEmpty()) {
    332302            List<OsmPrimitive> duplicates = new ArrayList<>();

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

    r10945 r11007  
    99import org.openstreetmap.josm.data.osm.DataSet;
    1010import org.openstreetmap.josm.data.osm.Node;
     11import org.openstreetmap.josm.data.osm.Tag;
     12import org.openstreetmap.josm.data.osm.Way;
     13import org.openstreetmap.josm.data.validation.TestError;
    1114import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
    1215import org.openstreetmap.josm.testutils.JOSMTestRules;
    … …  
    2629    public JOSMTestRules test = new JOSMTestRules();
    2730
     31    private static final DuplicateNode TEST = new DuplicateNode();
     32
     33    private static void doTest(int code, Tag ... tags) {
     34        performTest(code, buildDataSet(tags), true);
     35    }
     36
     37    private static void performTest(int code, DataSet ds, boolean fixable) {
     38        TEST.startTest(NullProgressMonitor.INSTANCE);
     39        TEST.visit(ds.allPrimitives());
     40        TEST.endTest();
     41
     42        assertEquals(1, TEST.getErrors().size());
     43        TestError error = TEST.getErrors().iterator().next();
     44        assertEquals(code, error.getCode());
     45        assertEquals(fixable, error.isFixable());
     46    }
     47
     48    private static DataSet buildDataSet(Tag... tags) {
     49        DataSet ds = new DataSet();
     50
     51        Node a = new Node(new LatLon(10.0, 5.0));
     52        Node b = new Node(new LatLon(10.0, 5.0));
     53        ds.addPrimitive(a);
     54        ds.addPrimitive(b);
     55
     56        if (tags.length > 0) {
     57            Way parent = new Way();
     58            parent.addNode(a);
     59            parent.addNode(b);
     60            for (Tag tag : tags) {
     61                parent.put(tag);
     62            }
     63            ds.addPrimitive(parent);
     64        }
     65        return ds;
     66    }
     67
    2868    /**
    29      * Test of "Duplicate node" validation test.
     69     * Test of "Duplicate node" validation test - different tag sets.
    3070     */
    3171    @Test
    … …  
    3878        ds.addPrimitive(b);
    3979
    40         DuplicateNode test = new DuplicateNode();
    41         test.startTest(NullProgressMonitor.INSTANCE);
    42         test.visit(ds.allPrimitives());
    43         test.endTest();
     80        a.put("foo", "bar");
     81        b.put("bar", "foo");
    4482
    45         assertEquals(1, test.getErrors().size());
     83        performTest(DuplicateNode.DUPLICATE_NODE, ds, false);
     84    }
     85
     86    /**
     87     * Test of "Duplicate node" validation test - mixed case.
     88     */
     89    @Test
     90    public void testDuplicateNodeMixed() {
     91        doTest(DuplicateNode.DUPLICATE_NODE_MIXED, new Tag("building", "foo"), new Tag("highway", "bar"));
     92    }
     93
     94    /**
     95     * Test of "Duplicate node" validation test - other case.
     96     */
     97    @Test
     98    public void testDuplicateNodeOther() {
     99        doTest(DuplicateNode.DUPLICATE_NODE_OTHER);
     100    }
     101
     102    /**
     103     * Test of "Duplicate node" validation test - building case.
     104     */
     105    @Test
     106    public void testDuplicateNodeBuilding() {
     107        doTest(DuplicateNode.DUPLICATE_NODE_BUILDING, new Tag("building", "foo"));
     108    }
     109
     110    /**
     111     * Test of "Duplicate node" validation test - boundary case.
     112     */
     113    @Test
     114    public void testDuplicateNodeBoundary() {
     115        doTest(DuplicateNode.DUPLICATE_NODE_BOUNDARY, new Tag("boundary", "foo"));
     116    }
     117
     118    /**
     119     * Test of "Duplicate node" validation test - highway case.
     120     */
     121    @Test
     122    public void testDuplicateNodeHighway() {
     123        doTest(DuplicateNode.DUPLICATE_NODE_HIGHWAY, new Tag("highway", "foo"));
     124    }
     125
     126    /**
     127     * Test of "Duplicate node" validation test - landuse case.
     128     */
     129    @Test
     130    public void testDuplicateNodeLanduse() {
     131        doTest(DuplicateNode.DUPLICATE_NODE_LANDUSE, new Tag("landuse", "foo"));
     132    }
     133
     134    /**
     135     * Test of "Duplicate node" validation test - natural case.
     136     */
     137    @Test
     138    public void testDuplicateNodeNatural() {
     139        doTest(DuplicateNode.DUPLICATE_NODE_NATURAL, new Tag("natural", "foo"));
     140    }
     141
     142    /**
     143     * Test of "Duplicate node" validation test - power case.
     144     */
     145    @Test
     146    public void testDuplicateNodePower() {
     147        doTest(DuplicateNode.DUPLICATE_NODE_POWER, new Tag("power", "foo"));
     148    }
     149
     150    /**
     151     * Test of "Duplicate node" validation test - railway case.
     152     */
     153    @Test
     154    public void testDuplicateNodeRailway() {
     155        doTest(DuplicateNode.DUPLICATE_NODE_RAILWAY, new Tag("railway", "foo"));
     156    }
     157
     158    /**
     159     * Test of "Duplicate node" validation test - waterway case.
     160     */
     161    @Test
     162    public void testDuplicateNodeWaterway() {
     163        doTest(DuplicateNode.DUPLICATE_NODE_WATERWAY, new Tag("waterway", "foo"));
    46164    }
    47165}
Note: See TracChangeset for help on using the changeset viewer.