Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add geojson parsing support #655

Merged
merged 2 commits into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,16 @@ sourceCompatibility = '1.8'

dependencies {
// Be sure to update dependencies in pom.xml to match
implementation 'joda-time:joda-time:2.10.10'
implementation 'org.slf4j:slf4j-api:1.7.31'
implementation 'joda-time:joda-time:2.10.13'
implementation 'org.slf4j:slf4j-api:1.7.33'
lognaturel marked this conversation as resolved.
Show resolved Hide resolved
implementation 'com.fasterxml.jackson.core:jackson-databind:2.13.1'

// Upgrade to version higher than 1.4 when Collect minSDK >= 26
implementation 'org.apache.commons:commons-csv:1.4'

compileOnly 'net.sf.kxml:kxml2:2.3.0'
testImplementation 'ch.qos.logback:logback-classic:1.2.3'

testImplementation 'ch.qos.logback:logback-classic:1.2.10'
testImplementation 'junit:junit:4.13.2'
testImplementation 'net.sf.kxml:kxml2:2.3.0'
testImplementation 'org.hamcrest:hamcrest-all:1.3'
Expand Down
Binary file modified javarosa-android-api-level-checker.zip
Binary file not shown.
11 changes: 8 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,14 @@
<dependency>
<groupId>joda-time</groupId>
<artifactId>joda-time</artifactId>
<version>2.10.10</version>
<version>2.10.13</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.13.1</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-csv</artifactId>
Expand All @@ -64,12 +69,12 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>1.7.31</version>
<version>1.7.33</version>
</dependency>
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
<version>1.2.3</version>
<version>1.2.10</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.io.DataOutputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import org.javarosa.core.model.instance.geojson.GeoJsonExternalInstance;
import org.javarosa.core.reference.InvalidReferenceException;
import org.javarosa.core.reference.ReferenceManager;
import org.javarosa.core.util.externalizable.DeserializationException;
Expand Down Expand Up @@ -68,10 +69,12 @@ public static ExternalDataInstance build(String instanceSrc, String instanceId)
return new ExternalDataInstance(root, instanceId, instanceSrc);
}

private static TreeElement parseExternalInstance(String instanceSrc, String instanceId) throws IOException, InvalidReferenceException, InvalidStructureException, XmlPullParserException, UnfullfilledRequirementsException {
private static TreeElement parseExternalInstance(String instanceSrc, String instanceId)
throws IOException, InvalidReferenceException, InvalidStructureException, XmlPullParserException, UnfullfilledRequirementsException {
String path = getPath(instanceSrc);
return instanceSrc.contains("file-csv") ?
CsvExternalInstance.parse(instanceId, path) : XmlExternalInstance.parse(instanceId, path);
return instanceSrc.contains("file-csv") ? CsvExternalInstance.parse(instanceId, path)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered pulling this out into a more conventional factory so it could be tested but I don't see a big benefit given how everything else is structured at the moment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One problem here is that it doesn't look like the new case is covered by any tests. Do we maybe want an outside-in/black-box/smoke test level test for this feature to check everything is in place?

: instanceSrc.endsWith("geojson") ? GeoJsonExternalInstance.parse(instanceId, path)
: XmlExternalInstance.parse(instanceId, path);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright 2022 ODK
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.javarosa.core.model.instance.geojson;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
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 {
lognaturel marked this conversation as resolved.
Show resolved Hide resolved
final TreeElement root = new TreeElement("root", 0);
root.setInstanceName(instanceId);

ObjectMapper objectMapper = new ObjectMapper();
try (JsonParser jsonParser = objectMapper.getFactory().createParser(geojsonStream)) {
validatePreamble(jsonParser);

int multiplicity = 0;
while (jsonParser.nextToken() != JsonToken.END_ARRAY) {
GeojsonFeature feature = objectMapper.readValue(jsonParser, GeojsonFeature.class);

root.addChild(feature.toTreeElement(multiplicity));
multiplicity++;
}
}

return root;
}

private static void validatePreamble(JsonParser jsonParser) throws IOException {
if (jsonParser.nextToken() != JsonToken.START_OBJECT) {
throw new IOException("GeoJSON file must contain a top-level Object");
lognaturel marked this conversation as resolved.
Show resolved Hide resolved
}

jsonParser.nextToken();
String propertyName = jsonParser.getCurrentName();
jsonParser.nextToken();
if (!(propertyName.equals("type") && jsonParser.getValueAsString().equals("FeatureCollection"))) {
throw new IOException("GeoJSON file must contain a top-level FeatureCollection");
}

jsonParser.nextToken();
propertyName = jsonParser.getCurrentName();

if (!(propertyName.equals("features") && jsonParser.nextToken() == JsonToken.START_ARRAY)) {
throw new IOException("GeoJSON FeatureCollection must contain an array of features");
lognaturel marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright 2022 ODK
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.javarosa.core.model.instance.geojson;

import com.fasterxml.jackson.annotation.JsonAutoDetect;
import java.util.Map;
import org.javarosa.core.model.data.UncastData;
import org.javarosa.core.model.instance.TreeElement;

@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY)
public class GeojsonFeature {
private String type;
private GeojsonGeometry geometry;
private Map<String, String> properties;

public TreeElement toTreeElement(int multiplicity) {
TreeElement item = new TreeElement("item", multiplicity);

TreeElement geoField = new TreeElement("geometry", 0);
geoField.setValue(new UncastData(geometry.getOdkCoordinates()));
item.addChild(geoField);

for (String property : properties.keySet()) {
TreeElement field = new TreeElement(property, 0);
field.setValue(new UncastData(properties.get(property)));
item.addChild(field);
}

return item;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright 2022 ODK
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.javarosa.core.model.instance.geojson;

import com.fasterxml.jackson.annotation.JsonAutoDetect;
import java.util.ArrayList;

@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY)
public class GeojsonGeometry {
private String type;
private ArrayList<String> coordinates;

public String getType() {
return type;
}

public String getOdkCoordinates() {
if (!(type.equals("Point") && coordinates.size() == 2)) {
throw new UnsupportedOperationException("Only Points are currently supported");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to be a problem for long, but maybe we should just skip unsupported elements rather than exploding? That would let us test in Collect with .geojson files without worrying about it crashing the app.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we likely will have a release with only points. In that case it feels better to fail for reasons similar to the "fast external itemsets" bug you identified -- it's not great if a subset of user features are silently filtered out. I don't feel strongly about it but that was my reasoning for exploding.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Let's use a checked exception then, so there's not a risk of Collect missing this and it resulting in a crash.

}

return coordinates.get(0) + " " + coordinates.get(1) + " 0 0";
}
}
13 changes: 6 additions & 7 deletions src/main/java/org/javarosa/core/util/GeoUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,17 @@

package org.javarosa.core.util;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.List;

import static java.lang.Math.PI;
import static java.lang.Math.abs;
import static java.lang.Math.acos;
import static java.lang.Math.cos;
import static java.lang.Math.sin;
import static java.lang.Math.toRadians;
import static java.lang.Math.PI;

import java.util.ArrayList;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Author: Meletis Margaritis
Expand Down