Changeset 30575 in osm for applications

Timestamp:

2014-08-08T00:47:24+02:00 (10 years ago)

Author:

donvip

Message:

[josm_opendata] MIF:

• correct handling of charsets
• support of multi segments polylines
• improve unit tests

Location:

applications/editors/josm/plugins/opendata

Files:

• src/org/openstreetmap/josm/plugins/opendata/core/io/InputStreamReaderUnbuffered.java (added)
• src/org/openstreetmap/josm/plugins/opendata/core/io/geographic/AbstractMapInfoReader.java (modified) (2 diffs)
• src/org/openstreetmap/josm/plugins/opendata/core/io/geographic/MifReader.java (modified) (11 diffs)
• src/org/openstreetmap/josm/plugins/opendata/core/io/geographic/TabReader.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/plugins/opendata/core/io/NonRegFunctionalTests.java (modified) (2 diffs)

applications/editors/josm/plugins/opendata/src/org/openstreetmap/josm/plugins/opendata/core/io/geographic/AbstractMapInfoReader.java

@@ -4 +4 @@
 import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.nio.charset.Charset;
+import java.nio.charset.IllegalCharsetNameException;
+import java.nio.charset.UnsupportedCharsetException;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.List;
@@ -47 +48 @@
         }
         
-        protected final BufferedReader getDataReader(File headerFile, String extension, Charset charset) throws FileNotFoundException {
+        protected final BufferedReader getDataReader(File headerFile, String extension, Charset charset) throws IOException {
                 File dataFile = getDataFile(headerFile, extension);
-                return dataFile.exists() ? new BufferedReader(new InputStreamReader(new FileInputStream(dataFile), charset)) : null;
+                return dataFile.exists() ? Files.newBufferedReader(dataFile.toPath(), charset) : null;
         }
 
-        protected Charset parseCharset(String[] words) {
+        protected Charset parseCharset(String[] words) throws IllegalCharsetNameException, UnsupportedCharsetException {
                 return parseCharset(words, 1);
         }
 
-        protected Charset parseCharset(String[] words, int index) {
+        protected Charset parseCharset(String[] words, int index) throws IllegalCharsetNameException, UnsupportedCharsetException {
                 words[index] = words[index].replace("\"", "");
                 if (words[index].equalsIgnoreCase(CHARSET_WINDOWS_LATIN)) {

applications/editors/josm/plugins/opendata/src/org/openstreetmap/josm/plugins/opendata/core/io/geographic/MifReader.java

@@ -12 +12 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.io.Reader;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;
@@ -31 +32 @@
 import org.openstreetmap.josm.plugins.opendata.core.OdConstants;
 import org.openstreetmap.josm.plugins.opendata.core.datasets.AbstractDataSetHandler;
+import org.openstreetmap.josm.plugins.opendata.core.io.InputStreamReaderUnbuffered;
 import org.openstreetmap.josm.plugins.opendata.core.util.OdUtils;
 
@@ -48 +50 @@
         READING_POINTS,
         END_POLYGON,
-        START_POLYLINE,
+        START_POLYLINE_SEGMENT,
         END_POLYLINE
     }
-
+    
+    private File file;
+    private InputStream stream;
+    protected Charset charset;
     protected BufferedReader midReader;
 
@@ -86 +91 @@
     private int numpolygons = -1;
     private int numpts = -1;
+    
+    // PLine clause
+    private int numsections = -1;
     
     public static DataSet parseDataSet(InputStream in, File file,
@@ -281 +289 @@
         int index = parseAffineUnits(words);
         
-        // FIXME: no idea what projection has to be used for real with "non-earth" mode...
-        josmProj = Projections.getProjectionByCode("EPSG:4326"); // WGS 84
-        
         units = words[index+1];
         
@@ -317 +322 @@
         case "nonearth":
             parseCoordSysSyntax2(words);
+            
+            // Syntax2 is not meant to be used for maps, and still... # 9592 happened
+            // From MapInfo documentation:
+            // http://testdrive.mapinfo.com/TDC/mxtreme4java.nsf/22fbc128f401ad818525666a00646bda/50100fdbe3e0a85085256a770053be1a/$FILE/coordsys.txt
+            // Use syntax 1 (above) to explicitly define a coordinate system for an Earth map (a map having coordinates which are specified with respect to a 
+            // location on the surface of the Earth). The optional Projection parameters dictate what map projection, if any, should be used in conjunction with 
+            // the coordinate system. If the Projection clause is omitted, MapBasic uses a longitude, latitude coordinate system using the North American Datum of 1927 (NAD-27). 
+            // Use syntax 2 to explicitly define a non-Earth coordinate system, such as the coordinate system used in a floor plan or other CAD drawing. 
+            
+            // FIXME: allow user to choose projection ?
+            josmProj = new CustomProjection(null);
             break;
         case "layout":
         case "table":
         case "window":
-            // TODO: support Layout, Table, Window clauses 
-            Main.warn("TODO: "+line);
+            Main.error("Unsupported CoordSys clause: "+line);
             break;
         default:
-            Main.warn("Line "+lineNum+". Invalid CoordSys clause: "+line);
+            Main.error("Line "+lineNum+". Invalid CoordSys clause: "+line);
         }
     }
@@ -358 +373 @@
     }
     
-    private void startPolyLine() throws IOException {
+    private void startPolyLineSegment(boolean initial) throws IOException {
+        Way previousPolyline = polyline;
         polyline = new Way();
         ds.addPrimitive(polyline);
-        readAttributes(polyline);
+        if (initial) {
+            readAttributes(polyline);
+        } else if (previousPolyline != null) {
+            // Not sure about how to handle multiple segments. In doubt we create a new way with the same tags
+            polyline.setKeys(previousPolyline.getKeys());
+        }
         state = State.READING_POINTS;
     }
     
     private void parsePLine(String[] words) throws IOException {
+        numsections = 1;
         if (words.length <= 1 || "MULTIPLE".equalsIgnoreCase(words[1])) {
-            // TODO: pline with multiple sections
             numpts = -1;
-            state = State.START_POLYLINE;
+            state = State.START_POLYLINE_SEGMENT;
+            if (words.length >= 3) {
+                // pline with multiple sections
+                numsections = Integer.parseInt(words[2]);
+            }
         } else {
             numpts = Integer.parseInt(words[1]); // Not described in PDF but found in real files: PLINE XX, with XX = numpoints
-            startPolyLine();
+            startPolyLineSegment(true);
         }
     }
@@ -414 +439 @@
     }
 
-    private DataSet parse(InputStream in, File file, ProgressMonitor instance, Charset charset) throws IOException {
+    private void initializeReaders(InputStream in, File f, Charset cs, int bufSize) throws IOException {
+        stream = in;
+        charset = cs;
+        file = f;
+        Reader isr;
+        // Did you know ? new InputStreamReader(in, charset) has a non-configurable buffer of 8kb :(
+        if (bufSize < 8192) {
+            isr = new InputStreamReaderUnbuffered(in, charset);
+        } else {
+            isr = new InputStreamReader(in, charset);
+        }
+        headerReader = new BufferedReader(isr, bufSize);
+        if (midReader != null) {
+            midReader.close();
+        }
+        midReader = getDataReader(file, ".mid", charset);
+    }
+    
+    private DataSet parse(InputStream in, File file, ProgressMonitor instance, Charset cs) throws IOException {
         try {
-            headerReader = new BufferedReader(new InputStreamReader(in, charset));
-            midReader = getDataReader(file, ".mid", charset);
-            parseHeader();
-            if (midReader != null) {
-                midReader.close();
+            try {
+                // Read header byte per byte until we determine correct charset
+                initializeReaders(in, file, cs, 1);
+                parseHeader();
+                return ds;
+            } finally {
+                if (midReader != null) {
+                    midReader.close();
+                }
             }
-            return ds;
         } catch (UnsupportedEncodingException e) {
             throw new IOException(e);
@@ -433 +479 @@
             parseVersion(words);
         } else if (words[0].equalsIgnoreCase("Charset")) {
-            parseCharset(words);
+            // Reinitialize readers with an efficient buffer value now we know for sure the good charset
+            initializeReaders(stream, file, parseCharset(words), 8192);
         } else if (words[0].equalsIgnoreCase("Delimiter")) {
             parseDelimiter(words);
@@ -460 +507 @@
                 state = State.READING_POINTS;
                 
-            } else if (state == State.START_POLYLINE) {
+            } else if (state == State.START_POLYLINE_SEGMENT) {
                 numpts = Integer.parseInt(words[0]);
-                startPolyLine();
+                startPolyLineSegment(polyline != null);
                 
             } else if (state == State.READING_POINTS && numpts > 0) {
@@ -485 +532 @@
                         }
                     } else if (polyline != null) {
-                        state = State.UNKNOWN;
-                        polyline = null;
+                        if (--numsections > 0) {
+                            state = State.START_POLYLINE_SEGMENT;
+                        } else {
+                            state = State.UNKNOWN;
+                            polyline = null;
+                        }
                     }
                 }

applications/editors/josm/plugins/opendata/src/org/openstreetmap/josm/plugins/opendata/core/io/geographic/TabReader.java

@@ -8 +8 @@
 import java.io.InputStreamReader;
 import java.nio.charset.Charset;
+import java.nio.charset.IllegalCharsetNameException;
+import java.nio.charset.UnsupportedCharsetException;
 import java.util.ArrayList;
 import java.util.List;
 
-import org.geotools.data.shapefile.files.TabFiles;
 import org.geotools.data.shapefile.dbf.DbaseFileReader;
 import org.geotools.data.shapefile.dbf.DbaseFileReader.Row;
+import org.geotools.data.shapefile.files.TabFiles;
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
@@ -111 +113 @@
         }
 
-        private void parseType(String[] words) {
+        private void parseType(String[] words) throws IllegalCharsetNameException, UnsupportedCharsetException {
                 if (words[1].equalsIgnoreCase("NATIVE") && words[2].equalsIgnoreCase("Charset")) {
                         datCharset = parseCharset(words, 3);

applications/editors/josm/plugins/opendata/test/unit/org/openstreetmap/josm/plugins/opendata/core/io/NonRegFunctionalTests.java

@@ -16 +16 @@
 
 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;
@@ -32 +33 @@
         Collection<Node> nodes = ds.getNodes();
         assertFalse("No nodes in dataset for "+context, nodes.isEmpty());
-        // Nodes should all have coordinates
+        // Nodes should all have valid coordinates
         for (Node n : nodes) {
-            assertTrue("Node without coordinate found for "+context, n.getCoor() != null);
+            LatLon latlon = n.getCoor();
+            assertTrue("Node without coordinate found for "+context, latlon != null);
+            assertTrue("Node with invalid coordinate ("+latlon+") found for "+context, latlon.isValid());
         }
         // and no empty ways