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

Support CSV external data #397

Merged
merged 1 commit into from
Feb 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions resources/org/javarosa/xform/parse/external-select-csv.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<h:head>
<h:title>external select 10</h:title>
<model>
<itext>
<translation default="true()" lang="default">
<text id="static_instance-first-0">
<value>a</value>
</text>
<text id="static_instance-first-1">
<value>b</value>
</text>
<text id="static_instance-second-0">
<value>aa</value>
</text>
<text id="static_instance-second-1">
<value>ab</value>
</text>
<text id="static_instance-second-3">
<value>ba</value>
</text>
<text id="static_instance-second-4">
<value>bb</value>
</text>
</translation>
</itext>
<instance>
<external_select_10 id="external_select_10">
<first/>
<second/>
<meta>
<instanceID/>
</meta>
</external_select_10>
</instance>
<instance id="first">
<root>
<item>
<itextId>static_instance-first-0</itextId>
<name>a</name>
</item>
<item>
<itextId>static_instance-first-1</itextId>
<name>b</name>
</item>
</root>
</instance>
<instance id="second" src="jr://file-csv/external-select.csv">
</instance>
<bind nodeset="/external_select_10/first" type="select1"/>
<bind nodeset="/external_select_10/second" type="select1"/>
<bind calculate="concat('uuid:', uuid())" nodeset="/external_select_10/meta/instanceID" readonly="true()" type="string"/>
</model>
</h:head>
<h:body>
<select1 ref="/external_select_10/first">
<label>First</label>
<item>
<label>a</label>
<value>a</value>
</item>
<item>
<label>b</label>
<value>b</value>
</item>
</select1>
<select1 ref="/external_select_10/second">
<label>Second</label>
<itemset nodeset="instance('second')/root/item[first= /external_select_10/first ]">
<value ref="name"/>
<label ref="jr:itext(itextId)"/>
</itemset>
</select1>
</h:body>
</h:html>
5 changes: 5 additions & 0 deletions resources/org/javarosa/xform/parse/external-select.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
itextId,name,first
static_instance-second-0,aa,a
static_instance-second-0,ab,a
static_instance-second-0,ba,b
static_instance-second-0,bb,b
32 changes: 32 additions & 0 deletions src/org/javarosa/core/model/instance/CsvExternalInstance.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package org.javarosa.core.model.instance;

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;
import org.javarosa.core.model.data.UncastData;

public class CsvExternalInstance {
public static TreeElement parse(String instanceId, String path) throws IOException {
TreeElement root = new TreeElement("root", 0);
root.setInstanceName(instanceId);
BufferedReader br = new BufferedReader(new FileReader(path));
String csvLine = br.readLine();

if (csvLine != null) {
String[] fieldNames = csvLine.split(",");

while ((csvLine = br.readLine()) != null) {
TreeElement item = new TreeElement("item", 0);
String[] data = csvLine.split(",");
for (int i = 0; i < fieldNames.length; ++i) {
TreeElement field = new TreeElement(fieldNames[i], 0);
field.setValue(new UncastData(data[i]));
item.addChild(field);
}

root.addChild(item);
}
}
return root;
}
}
27 changes: 13 additions & 14 deletions src/org/javarosa/core/model/instance/ExternalDataInstance.java
Original file line number Diff line number Diff line change
@@ -1,24 +1,20 @@
package org.javarosa.core.model.instance;

import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import org.javarosa.core.reference.InvalidReferenceException;
import org.javarosa.core.reference.ReferenceManager;
import org.javarosa.core.util.externalizable.DeserializationException;
import org.javarosa.core.util.externalizable.ExtUtil;
import org.javarosa.core.util.externalizable.PrototypeFactory;
import org.javarosa.xml.ElementParser;
import org.javarosa.xml.TreeElementParser;
import org.javarosa.xml.util.InvalidStructureException;
import org.javarosa.xml.util.UnfullfilledRequirementsException;
import org.kxml2.io.KXmlParser;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xmlpull.v1.XmlPullParserException;

import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.FileInputStream;
import java.io.IOException;

// This is still a work in progress.

public class ExternalDataInstance extends DataInstance {
Expand All @@ -28,7 +24,9 @@ public class ExternalDataInstance extends DataInstance {

// todo Make @mdudzinski’s recommended changes from https://github.com/opendatakit/javarosa/pull/154#pullrequestreview-51806826

/** No-args constructor for deserialization */
/**
* No-args constructor for deserialization
*/
public ExternalDataInstance() {
}

Expand All @@ -42,8 +40,8 @@ private ExternalDataInstance(TreeElement root, String instanceId, String path) {
/**
* Builds an ExternalDataInstance
*
* @param instanceSrc the value of the instance’s src attribute, e.g., jr://file/…
* @param instanceId the ID of the new instance
* @param instanceSrc the value of the instance’s src attribute, e.g., jr://file/…
* @param instanceId the ID of the new instance
* @return a new ExternalDataInstance
* @throws IOException if FileInputStream can’t find the file, or ElementParser can’t read the stream
* @throws InvalidReferenceException if the ReferenceManager in getPath(String srcLocation) can’t derive a reference
Expand All @@ -58,9 +56,9 @@ public static ExternalDataInstance build(String instanceSrc, String instanceId)
}

private static TreeElement parseExternalInstance(String instanceSrc, String instanceId) throws IOException, InvalidReferenceException, InvalidStructureException, XmlPullParserException, UnfullfilledRequirementsException {
KXmlParser xmlParser = ElementParser.instantiateParser(new FileInputStream(getPath(instanceSrc)));
TreeElementParser treeElementParser = new TreeElementParser(xmlParser, 0, instanceId);
return treeElementParser.parse();
String path = getPath(instanceSrc);
return instanceSrc.contains("file-csv") ?
CsvExternalInstance.parse(instanceId, path) : XmlExternalInstance.parse(instanceId, path);
}

@Override
Expand Down Expand Up @@ -88,7 +86,7 @@ public void initialize(InstanceInitializationFactory initializer, String instanc

@Override
public void readExternal(DataInputStream in, PrototypeFactory pf)
throws IOException, DeserializationException {
throws IOException, DeserializationException {
super.readExternal(in, pf);
path = ExtUtil.readString(in);
try {
Expand All @@ -106,6 +104,7 @@ public void writeExternal(DataOutputStream out) throws IOException {

/**
* Returns the path of the URI at srcLocation.
*
* @param srcLocation the value of the <code>src</code> attribute of the <code>instance</code> element
*/
private static String getPath(String srcLocation) throws InvalidReferenceException {
Expand Down
20 changes: 20 additions & 0 deletions src/org/javarosa/core/model/instance/XmlExternalInstance.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package org.javarosa.core.model.instance;

import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import org.javarosa.xml.ElementParser;
import org.javarosa.xml.TreeElementParser;
import org.javarosa.xml.util.InvalidStructureException;
import org.javarosa.xml.util.UnfullfilledRequirementsException;
import org.kxml2.io.KXmlParser;
import org.xmlpull.v1.XmlPullParserException;

public class XmlExternalInstance {
public static TreeElement parse(String instanceId, String path) throws IOException, InvalidStructureException, XmlPullParserException, UnfullfilledRequirementsException {
InputStream inputStream = new FileInputStream(path);
KXmlParser xmlParser = ElementParser.instantiateParser(inputStream);
TreeElementParser treeElementParser = new TreeElementParser(xmlParser, 0, instanceId);
return treeElementParser.parse();
}
}
2 changes: 1 addition & 1 deletion src/org/javarosa/xform/parse/XFormParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ private void parseDoc(Map<String, String> namespacePrefixesByUri) {
final String instanceId = instanceNodeIdStrs.get(instanceIndex);
final String instanceSrc = instance.getAttributeValue(null, "src");

if (instanceSrc != null && instanceSrc.toLowerCase().startsWith("jr://file/")) {
if (instanceSrc != null && instanceSrc.toLowerCase().startsWith("jr://file")) { // file or file-csv
final ExternalDataInstance externalDataInstance;
try {
externalDataInstance = ExternalDataInstance.build(instanceSrc, instanceId);
Expand Down
10 changes: 1 addition & 9 deletions test/org/javarosa/core/reference/ReferenceManagerTest.java
Original file line number Diff line number Diff line change
@@ -1,22 +1,14 @@
package org.javarosa.core.reference;

import static org.hamcrest.Matchers.is;
import static org.javarosa.core.reference.ReferenceManagerTestUtils.buildReferenceFactory;
import static org.junit.Assert.assertThat;

import org.junit.Before;
import org.junit.Test;

public class ReferenceManagerTest {

public static PrefixedRootFactory buildReferenceFactory(String scheme, final String path) {
return new PrefixedRootFactory(new String[]{scheme + "/"}) {
@Override
protected Reference factory(String terminal, String URI) {
return new ResourceReference(path + "/" + terminal);
}
};
}

private ReferenceManager refManager = ReferenceManager.instance();

@Before
Expand Down
40 changes: 40 additions & 0 deletions test/org/javarosa/core/reference/ReferenceManagerTestUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package org.javarosa.core.reference;

import java.nio.file.Path;

public class ReferenceManagerTestUtils {
/**
* Returns a new ReferenceFactory that will derive (resolve) the given scheme to the given
* path.
*/
public static PrefixedRootFactory buildReferenceFactory(String scheme, final String path) {
return new PrefixedRootFactory(new String[]{scheme + "/"}) {
@Override
protected Reference factory(String terminal, String URI) {
return new ResourceReference(path + "/" + terminal);
}
};
}

/**
* Sets up the ReferenceManager singleton to derive (resolve) the given schema to the
* given path.
* <p>
* Please, be aware that this method resets the singleton ReferenceManager, which could
* have unintended consequences for other classes using it during the same JVM session.
* <p>
* Use of this method is intended when only one scheme is to be derived. If your test
* form uses more than one scheme, you will have to follow a more conventional setup of
* having a reference factory for jr://file to a base path and some session translators
* that derive any other scheme (e.g. jr://audio) to a jr://file path.
*/
public static ReferenceManager setUpSimpleReferenceManager(String scheme, Path path) {
ReferenceManager refManager = ReferenceManager.instance();
refManager.reset();
refManager.addReferenceFactory(buildReferenceFactory(
scheme,
path.toAbsolutePath().toString()
));
return refManager;
}
}
20 changes: 17 additions & 3 deletions test/org/javarosa/xform/parse/XFormParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.nio.file.Path;
import java.text.NumberFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -54,7 +55,8 @@
import static org.hamcrest.core.IsInstanceOf.instanceOf;
import static org.javarosa.core.model.Constants.CONTROL_RANGE;
import static org.javarosa.core.model.Constants.CONTROL_RANK;
import static org.javarosa.core.reference.ReferenceManagerTest.buildReferenceFactory;
import static org.javarosa.core.reference.ReferenceManagerTestUtils.buildReferenceFactory;
import static org.javarosa.core.reference.ReferenceManagerTestUtils.setUpSimpleReferenceManager;
import static org.javarosa.core.util.externalizable.ExtUtil.defaultPrototypes;
import static org.javarosa.test.utils.ResourcePathHelper.r;
import static org.javarosa.xform.parse.FormParserHelper.parse;
Expand Down Expand Up @@ -105,7 +107,11 @@ public void parsesForm2() throws IOException {

@Test
public void parsesPreloadForm() throws IOException {
FormDef formDef = parse(r("Sample-Preloading.xml"));
// The form on this test uses a jr://file-csv resource.
// We need to prime the ReferenceManager to deal with those
Path form = r("Sample-Preloading.xml");
Copy link
Member

Choose a reason for hiding this comment

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

@dcbriccetti @ggalmazor This particular form doesn't actually use the instance loaded by file-csv because it relies on the Collect-only database-backed implementation of the search() appearance/function: https://github.com/opendatakit/collect/blob/d52ce0bfc63a11fb7f3fc62e0c0fd7c58684527a/collect_app/src/main/java/org/odk/collect/android/external/handler/ExternalDataHandlerSearch.java. You'll see the form doesn't actually use the instance. It's not a big deal since this test only verifies that the form can be parsed but it would probably be worth changing to avoid confusion.

I would also recommend avoiding the "preload' word unless referring to https://opendatakit.github.io/xforms-spec/#preload-attributes

Copy link
Contributor

Choose a reason for hiding this comment

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

We should file an issue for this change

Copy link
Member

Choose a reason for hiding this comment

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

I propose removing that test in #452

setUpSimpleReferenceManager("file-csv", form.getParent());
FormDef formDef = parse(form);
assertEquals("Sample Form - Preloading", formDef.getTitle());
}

Expand Down Expand Up @@ -145,6 +151,13 @@ public void parsesPreloadForm() throws IOException {
assertEquals("external select 10", formDef.getTitle());
}

@Test public void parsesExternalSecondaryInstanceCsvForm() throws IOException {
Path formName = r("external-select-csv.xml");
mapFileToResourcePath(formName);
FormDef formDef = parse(formName);
assertEquals("external select 10", formDef.getTitle());
}

@Test public void timesParsingLargeInternalSecondaryInstanceFiles() throws IOException {
timeParsing(new LargeIsiFileGenerator(SECONDARY_INSTANCE_XML), SECONDARY_INSTANCE_LARGE_XML,
SECONDARY_INSTANCE_LARGE_XML);
Expand Down Expand Up @@ -437,7 +450,8 @@ private void assertNoParseErrors(FormDef formDef) {
private void mapFileToResourcePath(Path formPath) {
ReferenceManager rm = ReferenceManager.instance();
rm.reset();
rm.addReferenceFactory(buildReferenceFactory("file", formPath.getParent().toString()));
for (String t : Arrays.asList("file", "file-csv"))
rm.addReferenceFactory(buildReferenceFactory(t, formPath.getParent().toString()));
}

/** Generates large versions of a secondary instance */
Expand Down