Index: trunk/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java =================================================================== --- trunk/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java (revision 11434) +++ trunk/src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java (revision 11435) @@ -538,5 +538,5 @@ public void put(String key, String value) { Map originalKeys = getKeys(); - if (key == null || Utils.strip(key).isEmpty()) + if (key == null || Utils.isStripEmpty(key)) return; else if (value == null) { Index: trunk/src/org/openstreetmap/josm/gui/history/VersionInfoPanel.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/history/VersionInfoPanel.java (revision 11434) +++ trunk/src/org/openstreetmap/josm/gui/history/VersionInfoPanel.java (revision 11435) @@ -274,5 +274,5 @@ // Update text, hide prefixing label if empty if (label != null) { - label.setVisible(text != null && !Utils.strip(text).isEmpty()); + label.setVisible(text != null && !Utils.isStripEmpty(text)); } textArea.setText(text); Index: trunk/src/org/openstreetmap/josm/gui/preferences/SourceEditor.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/preferences/SourceEditor.java (revision 11434) +++ trunk/src/org/openstreetmap/josm/gui/preferences/SourceEditor.java (revision 11435) @@ -965,5 +965,5 @@ private void updateOkButtonState() { - buttons.get(0).setEnabled(!Utils.strip(tfURL.getText()).isEmpty()); + buttons.get(0).setEnabled(!Utils.isStripEmpty(tfURL.getText())); } Index: trunk/src/org/openstreetmap/josm/io/OsmReader.java =================================================================== --- trunk/src/org/openstreetmap/josm/io/OsmReader.java (revision 11434) +++ trunk/src/org/openstreetmap/josm/io/OsmReader.java (revision 11435) @@ -25,4 +25,5 @@ import org.openstreetmap.josm.data.DataSource; import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.AbstractPrimitive; import org.openstreetmap.josm.data.osm.Changeset; import org.openstreetmap.josm.data.osm.DataSet; @@ -41,4 +42,5 @@ import org.openstreetmap.josm.gui.progress.ProgressMonitor; import org.openstreetmap.josm.tools.CheckParameterUtil; +import org.openstreetmap.josm.tools.Utils; import org.openstreetmap.josm.tools.date.DateUtils; @@ -377,4 +379,8 @@ if (key == null || value == null) { throwException(tr("Missing key or value attribute in tag.")); + } else if (Utils.isStripEmpty(key) && t instanceof AbstractPrimitive) { + // #14199: Empty keys as ignored by AbstractPrimitive#put, but it causes problems to fix existing data + // Drop the tag on import, but flag the primitive as modified + ((AbstractPrimitive) t).setModified(true); } else { t.put(key.intern(), value.intern()); Index: trunk/src/org/openstreetmap/josm/tools/Utils.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/Utils.java (revision 11434) +++ trunk/src/org/openstreetmap/josm/tools/Utils.java (revision 11435) @@ -735,4 +735,22 @@ /** + * Determines if the given String would be empty if stripped. + * This is an efficient alternative to {@code strip(s).isEmpty()} that avoids to create useless String object. + * @param str The string to test + * @return {@code true} if the stripped version of {@code s} would be empty. + * @since 11435 + */ + public static boolean isStripEmpty(String str) { + if (str != null) { + for (int i = 0; i < str.length(); i++) { + if (!isStrippedChar(str.charAt(i), DEFAULT_STRIP)) { + return false; + } + } + } + return true; + } + + /** * An alternative to {@link String#trim()} to effectively remove all leading * and trailing white characters, including Unicode ones. @@ -774,6 +792,5 @@ boolean leadingSkipChar = true; while (leadingSkipChar && start < end) { - char c = str.charAt(start); - leadingSkipChar = Character.isWhitespace(c) || Character.isSpaceChar(c) || stripChar(skipChars, c); + leadingSkipChar = isStrippedChar(str.charAt(start), skipChars); if (leadingSkipChar) { start++; @@ -782,6 +799,5 @@ boolean trailingSkipChar = true; while (trailingSkipChar && end > start + 1) { - char c = str.charAt(end - 1); - trailingSkipChar = Character.isWhitespace(c) || Character.isSpaceChar(c) || stripChar(skipChars, c); + trailingSkipChar = isStrippedChar(str.charAt(end - 1), skipChars); if (trailingSkipChar) { end--; @@ -790,4 +806,8 @@ return str.substring(start, end); + } + + private static boolean isStrippedChar(char c, final char ... skipChars) { + return Character.isWhitespace(c) || Character.isSpaceChar(c) || stripChar(skipChars, c); } Index: trunk/test/data/regress/14199/emptytag.osm =================================================================== --- trunk/test/data/regress/14199/emptytag.osm (revision 11435) +++ trunk/test/data/regress/14199/emptytag.osm (revision 11435) @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + + Index: trunk/test/unit/org/openstreetmap/josm/io/OsmReaderTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/io/OsmReaderTest.java (revision 11435) +++ trunk/test/unit/org/openstreetmap/josm/io/OsmReaderTest.java (revision 11435) @@ -0,0 +1,33 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.io; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.InputStream; + +import org.junit.Test; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.gui.progress.NullProgressMonitor; + +/** + * Unit tests of {@link OsmReader} class. + */ +public class OsmReaderTest { + + /** + * Non-regression test for Bug #14199. + * @throws Exception if any error occurs + */ + @Test + public void testTicket14199() throws Exception { + try (InputStream in = TestUtils.getRegressionDataStream(14199, "emptytag.osm")) { + Way w = OsmReader.parseDataSet(in, NullProgressMonitor.INSTANCE).getWays().iterator().next(); + assertEquals(1, w.getKeys().size()); + assertNull(w.get(" ")); + assertTrue(w.isModified()); + } + } +} Index: trunk/test/unit/org/openstreetmap/josm/tools/UtilsTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/tools/UtilsTest.java (revision 11434) +++ trunk/test/unit/org/openstreetmap/josm/tools/UtilsTest.java (revision 11435) @@ -3,4 +3,7 @@ import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -10,5 +13,4 @@ import java.util.Locale; -import org.junit.Assert; import org.junit.Rule; import org.junit.Test; @@ -53,26 +55,42 @@ "\u3000"; // IDEOGRAPHIC SPACE // CHECKSTYLE.ON: SingleSpaceSeparator - Assert.assertNull(Utils.strip(null)); - Assert.assertEquals("", Utils.strip("")); - Assert.assertEquals("", Utils.strip(" ")); - Assert.assertEquals("", Utils.strip(" ")); - Assert.assertEquals("", Utils.strip(" ")); - Assert.assertEquals("", Utils.strip(someWhite)); - Assert.assertEquals("a", Utils.strip("a")); - Assert.assertEquals("ab", Utils.strip("ab")); - Assert.assertEquals("abc", Utils.strip("abc")); - Assert.assertEquals("a", Utils.strip(" a")); - Assert.assertEquals("ab", Utils.strip(" ab")); - Assert.assertEquals("abc", Utils.strip(" abc")); - Assert.assertEquals("a", Utils.strip("a ")); - Assert.assertEquals("ab", Utils.strip("ab ")); - Assert.assertEquals("abc", Utils.strip("abc ")); - Assert.assertEquals("a", Utils.strip(someWhite+"a"+someWhite)); - Assert.assertEquals("ab", Utils.strip(someWhite+"ab"+someWhite)); - Assert.assertEquals("abc", Utils.strip(someWhite+"abc"+someWhite)); + assertNull(Utils.strip(null)); + assertEquals("", Utils.strip("")); + assertEquals("", Utils.strip(" ")); + assertEquals("", Utils.strip(" ")); + assertEquals("", Utils.strip(" ")); + assertEquals("", Utils.strip(someWhite)); + assertEquals("a", Utils.strip("a")); + assertEquals("ab", Utils.strip("ab")); + assertEquals("abc", Utils.strip("abc")); + assertEquals("a", Utils.strip(" a")); + assertEquals("ab", Utils.strip(" ab")); + assertEquals("abc", Utils.strip(" abc")); + assertEquals("a", Utils.strip("a ")); + assertEquals("ab", Utils.strip("ab ")); + assertEquals("abc", Utils.strip("abc ")); + assertEquals("a", Utils.strip(someWhite+"a"+someWhite)); + assertEquals("ab", Utils.strip(someWhite+"ab"+someWhite)); + assertEquals("abc", Utils.strip(someWhite+"abc"+someWhite)); // extended skip - Assert.assertEquals("a", Utils.strip("a", "b")); - Assert.assertEquals("b", Utils.strip("acbcac", "ac")); + assertEquals("a", Utils.strip("a", "b")); + assertEquals("b", Utils.strip("acbcac", "ac")); + } + + /** + * Test of {@link Utils#isStripEmpty} method. + */ + @Test + public void testIsStripEmpty() { + assertTrue(Utils.isStripEmpty(null)); + assertTrue(Utils.isStripEmpty("")); + assertTrue(Utils.isStripEmpty(" ")); + assertTrue(Utils.isStripEmpty(" ")); + assertFalse(Utils.isStripEmpty("a")); + assertFalse(Utils.isStripEmpty("foo")); + assertFalse(Utils.isStripEmpty(" foo")); + assertFalse(Utils.isStripEmpty("foo ")); + assertFalse(Utils.isStripEmpty(" foo ")); } @@ -82,11 +100,11 @@ @Test public void testToHexString() { - Assert.assertEquals("", Utils.toHexString(null)); - Assert.assertEquals("", Utils.toHexString(new byte[0])); - Assert.assertEquals("01", Utils.toHexString(new byte[]{0x1})); - Assert.assertEquals("0102", Utils.toHexString(new byte[]{0x1, 0x2})); - Assert.assertEquals("12", Utils.toHexString(new byte[]{0x12})); - Assert.assertEquals("127f", Utils.toHexString(new byte[]{0x12, 0x7f})); - Assert.assertEquals("fedc", Utils.toHexString(new byte[]{(byte) 0xfe, (byte) 0xdc})); + assertEquals("", Utils.toHexString(null)); + assertEquals("", Utils.toHexString(new byte[0])); + assertEquals("01", Utils.toHexString(new byte[]{0x1})); + assertEquals("0102", Utils.toHexString(new byte[]{0x1, 0x2})); + assertEquals("12", Utils.toHexString(new byte[]{0x12})); + assertEquals("127f", Utils.toHexString(new byte[]{0x12, 0x7f})); + assertEquals("fedc", Utils.toHexString(new byte[]{(byte) 0xfe, (byte) 0xdc})); }