Pull test geojson out, increase test coverage

src/main/java/org/javarosa/core/model/instance/geojson/GeoJsonExternalInstance.java

@@ -21,20 +21,15 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import org.javarosa.core.model.instance.TreeElement;
 
 public class GeoJsonExternalInstance {
     public static TreeElement parse(String instanceId, String path) throws IOException {
-        return parse(instanceId, new FileInputStream(path));
-    }
-
-    protected static TreeElement parse(String instanceId, InputStream geojsonStream) throws IOException {
         final TreeElement root = new TreeElement("root", 0);
         root.setInstanceName(instanceId);
 
         ObjectMapper objectMapper = new ObjectMapper();
-        try (JsonParser jsonParser = objectMapper.getFactory().createParser(geojsonStream)) {
+        try (JsonParser jsonParser = objectMapper.getFactory().createParser(new FileInputStream(path))) {
             validatePreamble(jsonParser);
 
             int multiplicity = 0;

src/main/java/org/javarosa/core/model/instance/geojson/GeojsonFeature.java

@@ -17,6 +17,7 @@
 package org.javarosa.core.model.instance.geojson;
 
 import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import java.io.IOException;
 import java.util.Map;
 import org.javarosa.core.model.data.UncastData;
 import org.javarosa.core.model.instance.TreeElement;
@@ -27,7 +28,7 @@ public class GeojsonFeature {
     private GeojsonGeometry geometry;
     private Map<String, String> properties;
 
-    public TreeElement toTreeElement(int multiplicity) {
+    public TreeElement toTreeElement(int multiplicity) throws IOException {
         TreeElement item = new TreeElement("item", multiplicity);
 
         TreeElement geoField = new TreeElement("geometry", 0);

src/main/java/org/javarosa/core/model/instance/geojson/GeojsonGeometry.java

@@ -17,6 +17,7 @@
 package org.javarosa.core.model.instance.geojson;
 
 import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import java.io.IOException;
 import java.util.ArrayList;
 
 @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY)
@@ -28,9 +29,9 @@ public String getType() {
         return type;
     }
 
-    public String getOdkCoordinates() {
-        if (!(type.equals("Point") && coordinates.size() == 2)) {
-            throw new UnsupportedOperationException("Only Points are currently supported");
+    public String getOdkCoordinates() throws IOException {
+        if (!(getType().equals("Point") && coordinates.size() == 2)) {
+            throw new IOException("Only Points are currently supported");
         }
 
         return coordinates.get(0) + " " + coordinates.get(1) + " 0 0";

src/test/java/org/javarosa/core/model/instance/geojson/GeoJsonExternalInstanceTest.java

@@ -18,42 +18,65 @@
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.javarosa.test.utils.ResourcePathHelper.r;
 import static org.junit.Assert.fail;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
-import java.io.ByteArrayInputStream;
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import org.javarosa.core.model.instance.TreeElement;
 import org.junit.Test;
 
 public class GeoJsonExternalInstanceTest {
     @Test
-    public void parse_throwsException_ifTopLevelElementNotFeatureCollection() {
+    public void parse_throwsException_ifNoTopLevelObject() {
         try {
-            GeoJsonExternalInstance.parse("id", new ByteArrayInputStream(GEOMETRY_COLLECTION_GEOJSON.getBytes(StandardCharsets.UTF_8)));
+            GeoJsonExternalInstance.parse("id", r("not-object.geojson").toString());
             fail("Exception expected");
         } catch (IOException e) {
             // expected
         }
     }
 
     @Test
-    public void parse_parsesMultipleFeatures() throws IOException {
-        TreeElement featureCollection = GeoJsonExternalInstance.parse("id", new ByteArrayInputStream(FEATURE_COLLECTION_GEOJSON.getBytes(StandardCharsets.UTF_8)));
-        assertThat(featureCollection.getNumChildren(), is(2));
+    public void parse_throwsException_ifTopLevelObjectTypeNotFeatureCollection() {
+        try {
+            GeoJsonExternalInstance.parse("id", r("invalid-type.geojson").toString());
+            fail("Exception expected");
+        } catch (IOException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void parse_throwsException_ifNoFeaturesArray() {
+        try {
+            GeoJsonExternalInstance.parse("id", r("bad-futures-collection.geojson").toString());
+            fail("Exception expected");
+        } catch (IOException e) {
+            // expected
+        }
+    }
+
+    @Test
+    public void parse_throwsException_ifFeaturesNotArray() {
+        try {
+            GeoJsonExternalInstance.parse("id", r("bad-features-not-array.geojson").toString());
+            fail("Exception expected");
+        } catch (IOException e) {
+            // expected
+        }
     }
 
     @Test
-    public void parse_addsGeometryAsChild() throws IOException {
-        TreeElement featureCollection = GeoJsonExternalInstance.parse("id", new ByteArrayInputStream(FEATURE_COLLECTION_GEOJSON.getBytes(StandardCharsets.UTF_8)));
+    public void parse_addsGeometriesAsChildren_forMultipleFeatures() throws IOException {
+        TreeElement featureCollection = GeoJsonExternalInstance.parse("id", r("feature-collection.geojson").toString());
+        assertThat(featureCollection.getNumChildren(), is(2));
         assertThat(featureCollection.getChildAt(0).getChild("geometry", 0).getValue().getValue(), is("102 0.5 0 0"));
         assertThat(featureCollection.getChildAt(1).getChild("geometry", 0).getValue().getValue(), is("104 0.5 0 0"));
     }
 
     @Test
     public void parse_addsAllOtherPropertiesAsChildren() throws IOException {
-        TreeElement featureCollection = GeoJsonExternalInstance.parse("id", new ByteArrayInputStream(FEATURE_COLLECTION_GEOJSON.getBytes(StandardCharsets.UTF_8)));
+        TreeElement featureCollection = GeoJsonExternalInstance.parse("id", r("feature-collection.geojson").toString());
         assertThat(featureCollection.getChildAt(0).getNumChildren(), is(4));
         assertThat(featureCollection.getChildAt(0).getChild("name", 0).getValue().getValue(), is("My cool point"));
 
@@ -62,90 +85,12 @@ public void parse_addsAllOtherPropertiesAsChildren() throws IOException {
     }
 
     @Test
-    public void getOdkCoordinates_convertsPointGeoJsonGeometry() throws IOException {
-        String point = "{ \"type\": \"Point\", \"coordinates\": [ 102, 0.5 ] }";
-        ObjectMapper objectMapper = new ObjectMapper();
-        GeojsonGeometry geometry = objectMapper.readValue(point, GeojsonGeometry.class);
-        assertThat(geometry.getOdkCoordinates(), is("102 0.5 0 0"));
-    }
-
-    @Test
-    public void getOdkCoordinates_throwsException_ifGeometryNotPoint() throws IOException {
-        String notPoint = "{\n" +
-            "               \"type\": \"LineString\",\n" +
-            "               \"coordinates\": [102.0, 0.0]\n" +
-            "           }";
-        ObjectMapper objectMapper = new ObjectMapper();
-        GeojsonGeometry geometry = objectMapper.readValue(notPoint, GeojsonGeometry.class);
-
+    public void parse_throwsException_whenGeometryNotPoint() {
         try {
-            geometry.getOdkCoordinates();
+            GeoJsonExternalInstance.parse("id", r("feature-collection-with-line.geojson").toString());
             fail("Exception expected");
-        } catch (UnsupportedOperationException e) {
+        } catch (IOException e) {
             // expected
         }
     }
-
-    private static final String GEOMETRY_COLLECTION_GEOJSON = "{\n" +
-        "    \"geometries\": [\n" +
-        "        {\n" +
-        "            \"coordinates\": [\n" +
-        "                [\n" +
-        "                    10.0,\n" +
-        "                    11.2\n" +
-        "                ],\n" +
-        "                [\n" +
-        "                    10.5,\n" +
-        "                    11.9\n" +
-        "                ]\n" +
-        "            ],\n" +
-        "            \"type\": \"Linestring\"\n" +
-        "        },\n" +
-        "        {\n" +
-        "            \"coordinates\": [\n" +
-        "                10.0,\n" +
-        "                20.0\n" +
-        "            ],\n" +
-        "            \"type\": \"Point\"\n" +
-        "        }\n" +
-        "    ],\n" +
-        "    \"type\": \"GeometryCollection\"\n" +
-        "}";
-
-    private static final String FEATURE_COLLECTION_GEOJSON = "{\n" +
-        "    \"type\": \"FeatureCollection\",\n" +
-        "    \"features\": [\n" +
-        "        {\n" +
-        "            \"type\": \"Feature\",\n" +
-        "            \"geometry\": {\n" +
-        "                \"type\": \"Point\",\n" +
-        "                \"coordinates\": [\n" +
-        "                    102,\n" +
-        "                    0.5\n" +
-        "                ]\n" +
-        "            },\n" +
-        "            \"properties\": {\n" +
-        "                \"id\": \"fs87b\",\n" +
-        "                \"name\": \"My cool point\",\n" +
-        "                \"foo\": \"bar\"\n" +
-        "            }\n" +
-        "        },\n" +
-        "        {\n" +
-        "            \"type\": \"Feature\",\n" +
-        "            \"geometry\": {\n" +
-        "                \"type\": \"Point\",\n" +
-        "                \"coordinates\": [\n" +
-        "                    104,\n" +
-        "                    0.5\n" +
-        "                ]\n" +
-        "            },\n" +
-        "            \"properties\": {\n" +
-        "                \"id\": \"67abie\",\n" +
-        "                \"name\": \"Your cool point\",\n" +
-        "                \"foo\": \"quux\",\n" +
-        "                \"special-property\": \"special value\"\n" +
-        "            }\n" +
-        "        }\n" +
-        "    ]\n" +
-        "}";
 }

src/test/java/org/javarosa/xform/parse/ExternalSecondaryInstanceParseTest.java

@@ -39,6 +39,8 @@
 import org.junit.Test;
 
 public class ExternalSecondaryInstanceParseTest {
+
+    //region Parsing of different file types into external secondary instances
     @Test
     public void itemsFromExternalSecondaryXMLInstance_ShouldBeAvailableToXPathParser() throws IOException, XPathSyntaxException {
         configureReferenceManagerCorrectly();
@@ -55,6 +57,39 @@ public void itemsFromExternalSecondaryXMLInstance_ShouldBeAvailableToXPathParser
         assertThat(fifthItem.getChild("label", 0).getValue().getDisplayText(), is("AB"));
     }
 
+    @Test
+    public void itemsFromExternalSecondaryGeoJsonInstance_ShouldBeAvailableToXPathParser() throws IOException, XPathSyntaxException {
+        configureReferenceManagerCorrectly();
+
+        FormDef formDef = parse(r("external-select-geojson.xml"));
+        assertEquals("GeoJSON External Secondary Instance", formDef.getTitle());
+
+        TreeReference treeReference = ((XPathPathExpr) parseXPath("instance('external-geojson')/root/item")).getReference();
+        EvaluationContext evaluationContext = formDef.getEvaluationContext();
+        List<TreeReference> treeReferences = evaluationContext.expandReference(treeReference);
+        assertThat(treeReferences.size(), is(2));
+
+        AbstractTreeElement secondItem = formDef.getNonMainInstance("external-geojson").resolveReference(treeReferences.get(1));
+        assertThat(secondItem.getChild("name", 0).getValue().getDisplayText(), is("Your cool point"));
+    }
+
+    @Test
+    public void itemsFromExternalSecondaryCSVInstance_ShouldBeAvailableToXPathParser() throws IOException, XPathSyntaxException {
+        configureReferenceManagerCorrectly();
+
+        FormDef formDef = parse(r("external-select-csv.xml"));
+        assertEquals("CSV External Secondary Instance", formDef.getTitle());
+
+        TreeReference treeReference = ((XPathPathExpr) parseXPath("instance('external-csv')/root/item")).getReference();
+        EvaluationContext evaluationContext = formDef.getEvaluationContext();
+        List<TreeReference> treeReferences = evaluationContext.expandReference(treeReference);
+        assertThat(treeReferences.size(), is(12));
+
+        AbstractTreeElement fifthItem = formDef.getNonMainInstance("external-csv").resolveReference(treeReferences.get(4));
+        assertThat(fifthItem.getChild("label", 0).getValue().getDisplayText(), is("AB"));
+    }
+    //endregion
+
     @Test
     public void xformParseException_whenItemsetConfiguresValueOrLabelNotInExternalInstance() throws IOException {
         configureReferenceManagerCorrectly();
@@ -108,22 +143,7 @@ public void deserializedFormDefCreatedFromAFormWithExternalSecondaryXMLInstance_
         assertTrue(deserializedFormDef.getFormInstances().containsKey("external-xml"));
     }
 
-    @Test
-    public void itemsFromExternalSecondaryCSVInstance_ShouldBeAvailableToXPathParser() throws IOException, XPathSyntaxException {
-        configureReferenceManagerCorrectly();
-
-        FormDef formDef = parse(r("external-select-csv.xml"));
-        assertEquals("CSV External Secondary Instance", formDef.getTitle());
-
-        TreeReference treeReference = ((XPathPathExpr) parseXPath("instance('external-csv')/root/item")).getReference();
-        EvaluationContext evaluationContext = formDef.getEvaluationContext();
-        List<TreeReference> treeReferences = evaluationContext.expandReference(treeReference);
-        assertThat(treeReferences.size(), is(12));
-
-        AbstractTreeElement fifthItem = formDef.getNonMainInstance("external-csv").resolveReference(treeReferences.get(4));
-        assertThat(fifthItem.getChild("label", 0).getValue().getDisplayText(), is("AB"));
-    }
-
+    //region ODK Collect database-driven external file features
     // ODK Collect has CSV-parsing features that bypass XPath and use databases. This test verifies that if a
     // secondary instance is declared but not referenced in an instance() call, it is ignored by JavaRosa.
     @Test
@@ -148,6 +168,7 @@ public void externalInstanceDeclaration_ShouldBeIgnored_WhenNotReferenced_AfterP
         formDef = fpi.getFormDef();
         assertThat(formDef.getNonMainInstance("external-csv"), nullValue());
     }
+    //endregion
 
     // See https://github.com/getodk/javarosa/issues/451
     @Test
@@ -161,6 +182,7 @@ public void dummyNodesInExternalInstanceDeclaration_ShouldBeIgnored() throws IOE
         assertThat(dataSet.size(), is(12));
     }
 
+    //region Missing external file
     @Test
     public void emptyPlaceholderInstanceIsUsed_whenExternalInstanceNotFound() {
         configureReferenceManagerIncorrectly();
@@ -232,6 +254,7 @@ public void exceptionFromChoiceSelection_whenFormIsDeserialized_afterPlaceholder
             // pass
         }
     }
+    //endregion
 
     // All external secondary instances and forms are in the same folder. Configure the ReferenceManager to resolve
     // URIs to that folder.

src/test/resources/org/javarosa/core/model/instance/geojson/bad-features-not-array.geojson

@@ -0,0 +1,4 @@
+{
+    "type": "FeatureCollection",
+    "features": "foo"
+}

src/test/resources/org/javarosa/core/model/instance/geojson/bad-futures-collection.geojson

@@ -0,0 +1,5 @@
+{
+    "type": "FeatureCollection",
+    "futures": [
+    ]
+}

src/test/resources/org/javarosa/core/model/instance/geojson/feature-collection-with-line.geojson

@@ -0,0 +1,20 @@
+{
+    "type": "FeatureCollection",
+    "features": [
+        {
+            "type": "Feature",
+            "geometry": {
+                "type": "LineString",
+                "coordinates": [
+                    102,
+                    0.5
+                ]
+            },
+            "properties": {
+                "id": "fs87b",
+                "name": "My cool line",
+                "foo": "bar"
+            }
+        }
+    ]
+}