Index: trunk/test/unit/org/openstreetmap/josm/data/validation/tests/TagCheckerTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/data/validation/tests/TagCheckerTest.java (revision 19200) +++ trunk/test/unit/org/openstreetmap/josm/data/validation/tests/TagCheckerTest.java (revision 19201) @@ -7,16 +7,26 @@ import java.io.IOException; +import java.text.MessageFormat; import java.util.Collections; import java.util.List; import java.util.function.Consumer; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmUtils; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.Tag; +import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper; import org.openstreetmap.josm.data.validation.Severity; @@ -44,153 +54,90 @@ checker.initialize(); checker.startTest(null); - checker.check(TestUtils.addFakeDataSet(primitive)); + if (primitive.getDataSet() == null) { + TestUtils.addFakeDataSet(primitive); + } + checker.check(primitive); return checker.getErrors(); } - /** - * Check for misspelled key. - * @throws IOException if any I/O error occurs - */ - @Test - void testMisspelledKey1() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node Name=Main")); + static Stream testTags() { + final String misspelled = "Misspelled property key"; + final String unknown = "Unknown property value"; + final String noPropertyValue = "Presets do not contain property value"; + final String looksLike = "Key ''{0}'' looks like ''{1}''."; + final String invalidPreset = "Key from a preset is invalid in this region"; + return Stream.of( + // Check for misspelled key. + Arguments.of("testMisspelledKey1", "node Name=Main", misspelled, + MessageFormat.format(looksLike, "Name", "name"), Severity.WARNING, true), + // Check for misspelled key. + Arguments.of("testMisspelledKey2", "node landuse;=forest", misspelled, + MessageFormat.format(looksLike, "landuse;", "landuse"), Severity.WARNING, true), + // Check for misspelled key where the suggested alternative is in use. The error should not be fixable. + Arguments.of("testMisspelledKeyButAlternativeInUse", "node amenity=fuel brand=bah Brand=foo", misspelled, + MessageFormat.format(looksLike, "Brand", "brand"), Severity.WARNING, false), + // Check for misspelled key where the suggested alternative is given with prefix E: in ignoreTags.cfg. + // The error should be fixable. + // ticket 17468 + Arguments.of("testUpperCaseIgnoredKey", "node wheelchair:Description=bla", misspelled, + MessageFormat.format(looksLike, "wheelchair:Description", "wheelchair:description"), Severity.WARNING, true), + // Check for misspelled key where the suggested alternative is given with prefix K: in ignoreTags.cfg. + // The error should be fixable. + // ticket 17468 + Arguments.of("testUpperCaseInKeyIgnoredTag", "node land_Area=administrative", misspelled, + MessageFormat.format(looksLike, "land_Area", "land_area"), Severity.WARNING, true), + // Check for unknown key + Arguments.of("testTranslatedNameKey", "node namez=Baz", + "Presets do not contain property key", "Key 'namez' not in presets.", Severity.OTHER, false), + // Check for misspelled value + Arguments.of("testMisspelledTag", "node landuse=forrest", unknown, + "Value 'forrest' for key 'landuse' is unknown, maybe 'forest' is meant?", Severity.WARNING, false), + // Check for misspelled value with multiple alternatives in presets. + Arguments.of("testMisspelledTag2", "node highway=servics", unknown, + "Value 'servics' for key 'highway' is unknown, maybe one of [service, services] is meant?", Severity.WARNING, false), + // Check for misspelled value. + Arguments.of("testMisspelledTag3", "node highway=residentail", unknown, + "Value 'residentail' for key 'highway' is unknown, maybe 'residential' is meant?", Severity.WARNING, false), + // Check for misspelled value. + Arguments.of("testShortValNotInPreset2", "node shop=abs", noPropertyValue, + "Value 'abs' for key 'shop' not in presets.", Severity.OTHER, false), + // Check regression: Don't fix surface=u -> surface=mud. + Arguments.of("testTooShortToFix", "node surface=u", noPropertyValue, + "Value 'u' for key 'surface' not in presets.", Severity.OTHER, false), + // Check value with upper case + Arguments.of("testValueDifferentCase", "node highway=Residential", unknown, + "Value 'Residential' for key 'highway' is unknown, maybe 'residential' is meant?", Severity.WARNING, false), + Arguments.of("testRegionKey", "node payment:ep_avant=yes", invalidPreset, + "Preset Payment Methods should not have the key payment:ep_avant", Severity.WARNING, false), + Arguments.of("testRegionTag", "relation type=waterway gnis:feature_id=123456", invalidPreset, + "Preset Waterway should not have the key gnis:feature_id", Severity.WARNING, false), + // Key in presets but not in ignored.cfg. Caused a NPE with r14727. + Arguments.of("testRegression17246", "node access=privat", unknown, + "Value 'privat' for key 'access' is unknown, maybe 'private' is meant?", Severity.WARNING, false) + ); + } + + /** + * Test tags + * @param name The name of the test + * @param primitive The primitive definition to use + * @param expectedMessage The expected error message + * @param expectedDescription The expected error description + * @param severity The expected severity + * @param isFixable {@code true} if the error should be fixable + * @throws IOException See {@link #test(OsmPrimitive)} + */ + @ParameterizedTest(name = "{0}") + @MethodSource("testTags") + void testTags(String name, String primitive, String expectedMessage, String expectedDescription, Severity severity, boolean isFixable) + throws IOException { + final List errors = test(OsmUtils.createPrimitive(primitive)); assertEquals(1, errors.size()); - assertEquals("Misspelled property key", errors.get(0).getMessage()); - assertEquals("Key 'Name' looks like 'name'.", errors.get(0).getDescription()); - assertTrue(errors.get(0).isFixable()); - } - - /** - * Check for misspelled key. - * @throws IOException if any I/O error occurs - */ - @Test - void testMisspelledKey2() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node landuse;=forest")); - assertEquals(1, errors.size()); - assertEquals("Misspelled property key", errors.get(0).getMessage()); - assertEquals("Key 'landuse;' looks like 'landuse'.", errors.get(0).getDescription()); - assertTrue(errors.get(0).isFixable()); - } - - /** - * Check for misspelled key where the suggested alternative is in use. The error should not be fixable. - * @throws IOException if any I/O error occurs - */ - @Test - void testMisspelledKeyButAlternativeInUse() throws IOException { - // ticket 12329 - final List errors = test(OsmUtils.createPrimitive("node amenity=fuel brand=bah Brand=foo")); - assertEquals(1, errors.size()); - assertEquals("Misspelled property key", errors.get(0).getMessage()); - assertEquals("Key 'Brand' looks like 'brand'.", errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } - - /** - * Check for misspelled key where the suggested alternative is given with prefix E: in ignoreTags.cfg. - * The error should be fixable. - * @throws IOException if any I/O error occurs - */ - @Test - void testUpperCaseIgnoredKey() throws IOException { - // ticket 17468 - final List errors = test(OsmUtils.createPrimitive("node wheelchair:Description=bla")); - assertEquals(1, errors.size()); - assertEquals("Misspelled property key", errors.get(0).getMessage()); - assertEquals("Key 'wheelchair:Description' looks like 'wheelchair:description'.", errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertTrue(errors.get(0).isFixable()); - } - - /** - * Check for misspelled key where the suggested alternative is given with prefix K: in ignoreTags.cfg. - * The error should be fixable. - * @throws IOException if any I/O error occurs - */ - @Test - void testUpperCaseInKeyIgnoredTag() throws IOException { - // ticket 17468 - final List errors = test(OsmUtils.createPrimitive("node land_Area=administrative")); - assertEquals(1, errors.size()); - assertEquals("Misspelled property key", errors.get(0).getMessage()); - assertEquals("Key 'land_Area' looks like 'land_area'.", errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertTrue(errors.get(0).isFixable()); - } - - /** - * Check for unknown key. - * @throws IOException if any I/O error occurs - */ - @Test - void testTranslatedNameKey() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node namez=Baz")); - assertEquals(1, errors.size()); - assertEquals("Presets do not contain property key", errors.get(0).getMessage()); - assertEquals("Key 'namez' not in presets.", errors.get(0).getDescription()); - assertEquals(Severity.OTHER, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } - - /** - * Check for misspelled value. - * @throws IOException if any I/O error occurs - */ - @Test - void testMisspelledTag() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node landuse=forrest")); - assertEquals(1, errors.size()); - assertEquals("Unknown property value", errors.get(0).getMessage()); - assertEquals("Value 'forrest' for key 'landuse' is unknown, maybe 'forest' is meant?", errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } - - /** - * Check for misspelled value with multiple alternatives in presets. - * @throws IOException if any I/O error occurs - */ - @Test - void testMisspelledTag2() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node highway=servics")); - assertEquals(1, errors.size()); - assertEquals("Unknown property value", errors.get(0).getMessage()); - assertEquals( - "Value 'servics' for key 'highway' is unknown, maybe one of [service, services] is meant?", - errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } - - /** - * Check for misspelled value. - * @throws IOException if any I/O error occurs - */ - @Test - void testMisspelledTag3() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node highway=residentail")); - assertEquals(1, errors.size()); - assertEquals("Unknown property value", errors.get(0).getMessage()); - assertEquals("Value 'residentail' for key 'highway' is unknown, maybe 'residential' is meant?", - errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } - - /** - * Check for misspelled value. - * @throws IOException if any I/O error occurs - */ - @Test - void testShortValNotInPreset2() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node shop=abs")); - assertEquals(1, errors.size()); - assertEquals("Presets do not contain property value", errors.get(0).getMessage()); - assertEquals("Value 'abs' for key 'shop' not in presets.", errors.get(0).getDescription()); - assertEquals(Severity.OTHER, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } + assertEquals(expectedMessage, errors.get(0).getMessage()); + assertEquals(expectedDescription, errors.get(0).getDescription()); + assertEquals(severity, errors.get(0).getSeverity()); + assertEquals(isFixable, errors.get(0).isFixable()); + } + /** @@ -206,69 +153,4 @@ .collect(Collectors.toList()); assertTrue(errors.isEmpty(), errors::toString); - } - - /** - * Check regression: Don't fix surface=u -> surface=mud. - * @throws IOException if any I/O error occurs - */ - @Test - void testTooShortToFix() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node surface=u")); - assertEquals(1, errors.size()); - assertEquals("Presets do not contain property value", errors.get(0).getMessage()); - assertEquals("Value 'u' for key 'surface' not in presets.", errors.get(0).getDescription()); - assertEquals(Severity.OTHER, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } - - /** - * Check value with upper case - * @throws IOException if any I/O error occurs - */ - @Test - void testValueDifferentCase() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node highway=Residential")); - assertEquals(1, errors.size()); - assertEquals("Unknown property value", errors.get(0).getMessage()); - assertEquals("Value 'Residential' for key 'highway' is unknown, maybe 'residential' is meant?", - errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } - - @Test - void testRegionKey() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node payment:ep_avant=yes")); - assertEquals(1, errors.size()); - assertEquals("Key from a preset is invalid in this region", errors.get(0).getMessage()); - assertEquals("Preset Payment Methods should not have the key payment:ep_avant", errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - - } - - @Test - void testRegionTag() throws IOException { - final List errors = test(OsmUtils.createPrimitive("relation type=waterway gnis:feature_id=123456")); - assertEquals(1, errors.size()); - assertEquals("Key from a preset is invalid in this region", errors.get(0).getMessage()); - assertEquals("Preset Waterway should not have the key gnis:feature_id", errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); - } - - /** - * Key in presets but not in ignored.cfg. Caused a NPE with r14727. - * @throws IOException if any I/O error occurs - */ - @Test - void testRegression17246() throws IOException { - final List errors = test(OsmUtils.createPrimitive("node access=privat")); - assertEquals(1, errors.size()); - assertEquals("Unknown property value", errors.get(0).getMessage()); - assertEquals("Value 'privat' for key 'access' is unknown, maybe 'private' is meant?", - errors.get(0).getDescription()); - assertEquals(Severity.WARNING, errors.get(0).getSeverity()); - assertFalse(errors.get(0).isFixable()); } @@ -388,5 +270,4 @@ */ @Test - @Disabled("broken, see #19519") void testTicket19519() throws IOException { List errors = test(OsmUtils.createPrimitive("node amenity=restaurant cuisine=bavarian;beef_bowl")); @@ -420,4 +301,29 @@ final List errors = test(OsmUtils.createPrimitive("node power=tower ref=12")); assertEquals(0, errors.size()); + } + + /** + * Non-regression test for Bug #23290 + */ + @Test + void testTicket23290() throws IOException { + final Node france1 = new Node(new LatLon(48.465638, 7.3677049)); + final Node france2 = new Node(new LatLon(48.4191768, 7.7275072)); + final Node german1 = new Node(new LatLon(48.3398223, 8.1683322)); + final Node german2 = new Node(new LatLon(48.4137076, 7.754287)); + final Way frenchRiver = TestUtils.newWay("waterway=river", france1, france2); + final Way germanRiver = TestUtils.newWay("waterway=river", german1, german2); + final Way incompleteWay = new Way(123, 0); + final Relation riverRelation = TestUtils.newRelation("type=waterway waterway=river ref:sandre=A---0000 ref:fgkz=2", + new RelationMember("", germanRiver)); + // Ensure they have the same dataset + new DataSet(france1, france2, frenchRiver, german1, german2, germanRiver, incompleteWay, riverRelation); + assertEquals(1, test(riverRelation).size()); + riverRelation.addMember(new RelationMember("", frenchRiver)); + assertEquals(0, test(riverRelation).size()); + riverRelation.removeMembersFor(frenchRiver); + assertEquals(1, test(riverRelation).size()); + riverRelation.addMember(new RelationMember("", incompleteWay)); + assertEquals(0, test(riverRelation).size()); }