Changeset 34898 in osm for applications

Timestamp:

2019-02-24T20:31:40+01:00 (6 years ago)

Author:

donvip

Message:

fix #josm13843 - Not properly loading attributes from multipart objects (patch by openbrian)

Location:

applications/editors/josm/plugins/opendata

Files:

• src/org/openstreetmap/josm/plugins/opendata/core/io/geographic/ShpReader.java (modified) (2 diffs)
• test/data/regress/12843 (added)
• test/data/regress/12843/test.dbf (added)
• test/data/regress/12843/test.prj (added)
• test/data/regress/12843/test.shp (added)
• test/data/regress/12843/test.shx (added)
• test/unit/org/openstreetmap/josm/plugins/opendata/core/io/geographic/ShpReaderTest.java (modified) (4 diffs)

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

@@ -116 +116 @@
             }
 
-            OsmPrimitive primitive = null;
-
-            if (geometry.getValue() instanceof Point) {
-                primitive = createOrGetEmptyNode((Point) geometry.getValue());
-
-            } else if (geometry.getValue() instanceof GeometryCollection) { // Deals with both MultiLineString and MultiPolygon
-                GeometryCollection mp = (GeometryCollection) geometry.getValue();
-                int nGeometries = mp.getNumGeometries();
-                if (nGeometries < 1) {
-                    Logging.error("empty geometry collection found");
-                } else {
-                    Relation r = null;
-                    Way w = null;
-
-                    for (int i = 0; i < nGeometries; i++) {
-                        Geometry g = mp.getGeometryN(i);
-                        if (g instanceof Polygon) {
-                            Polygon p = (Polygon) g;
-                            // Do not create relation if there's only one polygon without interior ring
-                            // except if handler prefers it
-                            if (r == null && (nGeometries > 1 || p.getNumInteriorRing() > 0 ||
-                                    (handler != null && handler.preferMultipolygonToSimpleWay()))) {
-                                r = createMultipolygon();
-                            }
-                            w = createOrGetWay(p.getExteriorRing());
-                            if (r != null) {
-                                addWayToMp(r, "outer", w);
-                                for (int j = 0; j < p.getNumInteriorRing(); j++) {
-                                    addWayToMp(r, "inner", createOrGetWay(p.getInteriorRingN(j)));
-                                }
-                            }
-                        } else if (g instanceof LineString) {
-                            w = createOrGetWay((LineString) g);
-                        } else if (g instanceof Point) {
-                            // Some belgian data sets hold points into collections ?!
-                            readNonGeometricAttributes(feature, createOrGetNode((Point) g));
-                        } else {
-                            Logging.error("unsupported geometry : "+g);
-                        }
-                    }
-                    primitive = r != null ? r : w;
+            Object geomObject = geometry.getValue();
+            if (geomObject instanceof Point) {  // TODO: Support LineString and Polygon.
+                // Sure you could have a Set of 1 object and join these 2 branches of
+                // code, but I feel there would be a performance hit.
+                OsmPrimitive primitive = createOrGetEmptyNode((Point) geomObject);
+                readNonGeometricAttributes(feature, primitive);
+            } else if (geomObject instanceof GeometryCollection) { // Deals with both MultiLineString and MultiPolygon
+                Set<OsmPrimitive> primitives = processGeometryCollection((GeometryCollection) geomObject);
+                for (OsmPrimitive prim : primitives) {
+                    readNonGeometricAttributes(feature, prim);
                 }
             } else {
@@ -164 +133 @@
                 Logging.debug("\tdescriptor: "+desc);
                 Logging.debug("\tname: "+geometry.getName());
-                Logging.debug("\tvalue: "+geometry.getValue());
+                Logging.debug("\tvalue: "+geomObject);
                 Logging.debug("\tid: "+geometry.getIdentifier());
                 Logging.debug("-------------------------------------------------------------");
             }
-
-            if (primitive != null) {
-                // Read primitive non geometric attributes
-                readNonGeometricAttributes(feature, primitive);
-            }
-        }
+        }
+    }
+
+    protected Set<OsmPrimitive> processGeometryCollection(GeometryCollection gc) throws TransformException {
+        // A feture may be a collection.  This set holds the items of the collection.
+        Set<OsmPrimitive> primitives = new HashSet<>();
+        int nGeometries = gc.getNumGeometries();
+        if (nGeometries < 1) {
+            Logging.error("empty geometry collection found");
+        } else {
+            // Create the primitive "op" and add it to the set of primitives.
+            for (int i = 0; i < nGeometries; i++) {
+                OsmPrimitive op = null;
+                Geometry g = gc.getGeometryN(i);
+                if (g instanceof Polygon) {
+                    Relation r = (Relation) op;
+                    Polygon p = (Polygon) g;
+                    // Do not create relation if there's only one polygon without interior ring
+                    // except if handler prefers it
+                    if (r == null && (nGeometries > 1 || p.getNumInteriorRing() > 0 ||
+                            (handler != null && handler.preferMultipolygonToSimpleWay()))) {
+                        r = createMultipolygon();
+                    }
+                    if (r != null) {
+                        addWayToMp(r, "outer", createOrGetWay(p.getExteriorRing()));
+                        for (int j = 0; j < p.getNumInteriorRing(); j++) {
+                            addWayToMp(r, "inner", createOrGetWay(p.getInteriorRingN(j)));
+                        }
+                    }
+                } else if (g instanceof LineString) {
+                    op = createOrGetWay((LineString) g);
+                } else if (g instanceof Point) {
+                    op = createOrGetNode((Point) g);
+                } else {
+                    Logging.error("unsupported geometry : "+g);
+                }
+                primitives.add(op);
+            }
+        }
+        return primitives;
     }
 

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

@@ -2 +2 @@
 package org.openstreetmap.josm.plugins.opendata.core.io.geographic;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -9 +10 @@
 import java.io.FileInputStream;
 import java.io.InputStream;
+import java.util.Collection;
 
 import org.junit.Ignore;
@@ -16 +18 @@
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.plugins.opendata.core.io.NonRegFunctionalTests;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
@@ -85 +88 @@
         }
     }
+
+    /**
+     * Non-regression test for ticket <a href="https://josm.openstreetmap.de/ticket/12843">#12843</a>
+     * @throws Exception if an error occurs during reading
+     */
+    @Test
+    public void testTicket12843() throws Exception {
+        File file = new File(TestUtils.getRegressionDataFile(12843, "test.shp"));
+        try (InputStream is = new FileInputStream(file)) {
+            Collection<Way> ways = ShpReader.parseDataSet(is, file, null, null).getWays();
+            assertFalse(ways.isEmpty());
+            for (Way way : ways) {
+                assertEquals("Test", way.get("name"));
+            }
+        }
+    }
 }