Changeset 6562 in josm

Timestamp:

2013-12-29T14:14:58+01:00 (11 years ago)

Author:

simon04

Message:

fix #9327 - tagging presets: fix handling of nested <reference>s, refactor railway presets

Location:

trunk

Files:

• data/defaultpresets.xml (modified) (10 diffs)
• src/org/openstreetmap/josm/gui/tagging/TaggingPresetReader.java (modified) (4 diffs)
• test/data/preset_chunk.xml (added)
• test/unit/org/openstreetmap/TestUtils.java (added)
• test/unit/org/openstreetmap/josm/gui/tagging/TaggingPresetReaderTest.java (modified) (4 diffs)

trunk/data/defaultpresets.xml

@@ -183 +183 @@
         <text key="width" text="Width (meters)" />
     </chunk>
+    <chunk id="railway_service">
+        <combo key="service" text="Service type" values="yard,siding,spur" />
+    </chunk>
+    <chunk id="railway_electrified">
+        <combo key="electrified" text="Electrified" values="contact_line,no,yes,rail" />
+        <combo key="voltage" text="Voltage in Volts (V)" values="600,650,750,1500,3000,15000,25000" length="5" />
+        <combo key="frequency" text="Frequency in Hertz (Hz)" values="0,16.67,16.7" length="5" />
+    </chunk>
+    <chunk id="railway_service_gauge_electrified">
+        <reference ref="railway_service" />
+        <combo key="gauge" text="Gauge (mm)" values="1668,1676,1674,1600,1524,1520,1495,1435,1067,1000,914,762,760,750" length="4" />
+        <reference ref="railway_electrified" />
+    </chunk>
     <chunk id="barrier_5">
         <optional text="Allowed traffic:">
@@ -1909 +1922 @@
             <optional>
                 <combo key="usage" text="Usage" values="main,branch,industrial,military,tourism,freight" />
-                <combo key="service" text="Service type" values="yard,siding,spur" />
-                <combo key="gauge" text="Gauge (mm)" values="1668,1676,1674,1600,1524,1520,1495,1435,1067,1000,914,762,760,750" length="4" />
-                <combo key="electrified" text="Electrified" values="contact_line,no,yes,rail" />
-                <combo key="voltage" text="Voltage in Volts (V)" values="600,650,750,1500,3000,15000,25000" length="5" />
-                <combo key="frequency" text="Frequency in Hertz (Hz)" values="0,16.67,16.7" length="5" />
+                <reference ref="railway_service_gauge_electrified" />
             </optional>
         </item>
@@ -1922 +1931 @@
             <optional>
                 <combo key="usage" text="Usage" values="main,branch,industrial,military,tourism,freight" />
-                <combo key="service" text="Service type" values="yard,siding,spur" />
-                <combo key="gauge" text="Gauge (mm)" values="1668,1676,1674,1600,1524,1520,1495,1435,1067,1000,914,762,760,750" length="4" />
-                <combo key="electrified" text="Electrified" values="contact_line,no,yes,rail" />
-                <combo key="voltage" text="Voltage in Volts (V)" values="600,650,750,1500,3000,15000,25000" length="5" />
-                <combo key="frequency" text="Frequency in Hertz (Hz)" values="0,16.67,16.7" length="5" />
+                <reference ref="railway_service_gauge_electrified" />
             </optional>
         </item>
@@ -1937 +1942 @@
             <key key="railway" value="monorail" />
             <optional>
-                <combo key="service" text="Service type" values="yard,siding,spur" />
-                <combo key="electrified" text="Electrified" values="contact_line,no,yes,rail" />
-                <combo key="voltage" text="Voltage in Volts (V)" values="600,650,750,1500,3000,15000,25000" length="5" />
-                <combo key="frequency" text="Frequency in Hertz (Hz)" values="0,16.67,16.7" length="5" />
+                <reference ref="railway_service" />
+                <reference ref="railway_electrified" />
             </optional>
         </item>
@@ -1948 +1951 @@
             <key key="railway" value="preserved" />
             <optional>
-                <combo key="service" text="Service type" values="yard,siding,spur" />
-                <combo key="gauge" text="Gauge (mm)" values="1668,1676,1674,1600,1524,1520,1495,1435,1067,1000,914,762,760,750" length="4" />
-                <combo key="electrified" text="Electrified" values="contact_line,no,yes,rail" />
-                <combo key="voltage" text="Voltage in Volts (V)" values="600,650,750,1500,3000,15000,25000" length="5" />
-                <combo key="frequency" text="Frequency in Hertz (Hz)" values="0,16.67,16.7" length="5" />
+                <reference ref="railway_service_gauge_electrified" />
             </optional>
         </item>
@@ -1960 +1959 @@
             <key key="railway" value="light_rail" />
             <optional>
-                <combo key="service" text="Service type" values="yard,siding,spur" />
-                <combo key="gauge" text="Gauge (mm)" values="1668,1676,1674,1600,1524,1520,1495,1435,1067,1000,914,762,760,750" length="4" />
-                <combo key="electrified" text="Electrified" values="contact_line,no,yes,rail" />
-                <combo key="voltage" text="Voltage in Volts (V)" values="600,650,750,1500,3000,15000,25000" length="5" />
-                <combo key="frequency" text="Frequency in Hertz (Hz)" values="0,16.67,16.7" length="5" />
+                <reference ref="railway_service_gauge_electrified" />
             </optional>
         </item>
@@ -1975 +1970 @@
             <key key="railway" value="subway" />
             <optional>
-                <combo key="service" text="Service type" values="yard,siding,spur" />
-                <combo key="gauge" text="Gauge (mm)" values="1668,1676,1674,1600,1524,1520,1495,1435,1067,1000,914,762,760,750" length="4" />
-                <combo key="electrified" text="Electrified" values="contact_line,no,yes,rail" />
-                <combo key="voltage" text="Voltage in Volts (V)" values="600,650,750,1500,3000,15000,25000" length="5" />
-                <combo key="frequency" text="Frequency in Hertz (Hz)" values="0,16.67,16.7" length="5" />
+                <reference ref="railway_service_gauge_electrified" />
             </optional>
         </item>
@@ -1992 +1983 @@
             <key key="railway" value="tram" />
             <optional>
-                <combo key="service" text="Service type" values="yard,siding,spur" />
-                <combo key="gauge" text="Gauge (mm)" values="1668,1676,1674,1600,1524,1520,1495,1435,1067,1000,914,762,760,750" length="4" />
-                <combo key="electrified" text="Electrified" values="contact_line,no,yes,rail" />
-                <combo key="voltage" text="Voltage in Volts (V)" values="600,650,750,1500,3000,15000,25000" length="5" />
-                <combo key="frequency" text="Frequency in Hertz (Hz)" values="0,16.67,16.7" length="5" />
+                <reference ref="railway_service_gauge_electrified" />
             </optional>
         </item>
@@ -2021 +2008 @@
             <key key="railway" value="disused" />
             <optional>
-                <combo key="service" text="Service type" values="yard,siding,spur" />
+                <reference ref="railway_service" />
             </optional>
         </item>
@@ -2030 +2017 @@
             <key key="railway" value="abandoned" />
             <optional>
-                <combo key="service" text="Service type" values="yard,siding,spur" />
+                <reference ref="railway_service" />
             </optional>
         </item>

trunk/src/org/openstreetmap/josm/gui/tagging/TaggingPresetReader.java

@@ -17 +17 @@
 import java.util.List;
 import java.util.Map;
+import java.util.Stack;
 
 import javax.swing.JOptionPane;
@@ -92 +93 @@
         List<TaggingPresetItems.PresetListEntry> listEntries = new LinkedList<TaggingPresetItems.PresetListEntry>();
         final Map<String, List<Object>> byId = new HashMap<String, List<Object>>();
-        String lastId = null;
-        Iterator<Object> lastIdIterator = null;
+        final Stack<String> lastIds = new Stack<String>();
+        /** lastIdIterators contains non empty iterators of items to be handled before obtaining the next item from the XML parser */
+        final Stack<Iterator<Object>> lastIdIterators = new Stack<Iterator<Object>>();
 
         if (validate) {
@@ -100 +102 @@
             parser.start(in);
         }
-        while (parser.hasNext()) {
+        while (parser.hasNext() || !lastIdIterators.isEmpty()) {
             final Object o;
-            if (lastIdIterator != null && lastIdIterator.hasNext()) {
-                // obtain elements from lastIdIterator with higher priority
-                o = lastIdIterator.next();
+            if (!lastIdIterators.isEmpty()) {
+                // obtain elements from lastIdIterators with higher priority
+                o = lastIdIterators.peek().next();
+                if (!lastIdIterators.peek().hasNext()) {
+                    // remove iterator is is empty
+                    lastIdIterators.pop();
+                }
             } else {
                 o = parser.next();
             }
             if (o instanceof Chunk) {
-                if (((Chunk) o).id.equals(lastId)) {
-                    // reset last id on end of object, don't process further
-                    lastId = null;
+                if (!lastIds.isEmpty() && ((Chunk) o).id.equals(lastIds.peek())) {
+                    // pop last id on end of object, don't process further
+                    lastIds.pop();
                     ((Chunk) o).id = null;
                     continue;
-                } else if (lastId == null) {
+                } else {
                     // if preset item contains an id, store a mapping for later usage
-                    lastId = ((Chunk) o).id;
+                    String lastId = ((Chunk) o).id;
+                    lastIds.push(lastId);
                     byId.put(lastId, new ArrayList<Object>());
                     continue;
-                } else {
-                    throw new IllegalStateException("Cannot deal with nested id objects (lastId was expected to be null)");
-                }
-            } else if (lastId != null) {
+                }
+            } else if (!lastIds.isEmpty()) {
                 // add object to mapping for later usage
-                byId.get(lastId).add(o);
+                byId.get(lastIds.peek()).add(o);
                 continue;
             }
@@ -134 +139 @@
                     throw new SAXException(tr("Reference {0} is being used before it was defined", ref));
                 }
-                lastIdIterator = byId.get(ref).iterator();
+                lastIdIterators.push(byId.get(ref).iterator());
                 continue;
             }

trunk/test/unit/org/openstreetmap/josm/gui/tagging/TaggingPresetReaderTest.java

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.gui.tagging;
-
-import java.io.IOException;
-import java.util.Collection;
 
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.openstreetmap.TestUtils;
 import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.data.Preferences;
+import org.openstreetmap.josm.tools.Utils;
 import org.xml.sax.SAXException;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
+
+import static org.CustomMatchers.hasSize;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
 
 /**
@@ -16 +22 @@
  */
 public class TaggingPresetReaderTest {
-    /**
-     * path to test data root directory
-     */
-    private static String testdataroot;
-    
+
     @BeforeClass
     public static void setUpClass() {
         Main.initApplicationPreferences();
-        testdataroot = System.getProperty("josm.test.data");
-        if (testdataroot == null || testdataroot.isEmpty()) {
-            testdataroot = "test/data";
-            System.out.println("System property josm.test.data is not set, using '" + testdataroot + "'");
-        }
     }
 
@@ -37 +34 @@
      */
     protected static String getRegressionDataDir(int ticketid) {
-        return testdataroot + "/regress/" + ticketid;
+        return TestUtils.getTestDataRoot() + "/regress/" + ticketid;
     }
 
@@ -64 +61 @@
     }
 
+    @Test
+    public void testNestedChunks() throws Exception {
+        final Collection<TaggingPreset> presets = TaggingPresetReader.readAll(TestUtils.getTestDataRoot() + "preset_chunk.xml", true);
+        assertThat(presets, hasSize(1));
+        final TaggingPreset abc =  presets.iterator().next();
+        final List<String> keys = Utils.transform(abc.data, new Utils.Function<TaggingPresetItem, String>() {
+            @Override
+            public String apply(TaggingPresetItem x) {
+                return ((TaggingPresetItems.Key) x).key;
+            }
+        });
+        assertThat(keys.toString(), is("[A1, A2, A3, B1, B2, B3, C1, C2, C3]"));
+    }
+
     /**
      * Validate internal presets