Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/org/opendatakit/validate/FormValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,30 @@ public Object eval(Object[] args, EvaluationContext ec) {
return args[0];
}});

fd.getEvaluationContext().addFunctionHandler(new IFunctionHandler() {

public String getName() {
return "intersects";
}

public List<Class[]> getPrototypes() {
return new ArrayList<Class[]>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't really see any extra risk in returning the real prototype here and not overriding rawArgs:

Arrays.asList(new Class[] { String.class })

In #100, you say:

I considered adding type checking but I have decided against it. The Collect implementation currently verifies that the shape of the data is a geotrace/shape.

That's true that Collect does extra verification as well, but it also just uses the JavaRosa prototype type check before it does any of that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because all types can be coerced to String, the only difference with doing that is that it would enforce a single argument. I'm on the fence about that. On one hand, it's nice to reject extra arguments now. On the other, it means that we have to remember to change the prototype as we add to intersects. We've talked about detecting intersections across shapes. I still lean towards leaving this lenient but if you still feel strongly after reading this I'll change it.

}

public boolean rawArgs() {
return true;
}

public boolean realTime() {
return false;
}

public Object eval(Object[] args, EvaluationContext ec) {
// stub for validation
return args[0];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this not return a Boolean?

@lognaturel lognaturel Jan 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

All of the built-in function handlers in JavaRosa return Object. I do think changing it to Boolean would be ok but my reasoning was to deviate as little as possible from the other functions. Again, if you still feel strongly about it I'll make the change.

I think you meant the actual return value, not the return type! I don't think it makes any practical difference because values would get coerced so I think it's safe to change.

}
});

// check for runtime errors
fd.initialize(true, new InstanceInitializationFactory());

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.opendatakit.validate;

import org.junit.Test;

import java.net.URISyntaxException;
import java.nio.file.Path;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.isEmptyString;

public class CollectFunctionHandlerTest {

@Test
public void acceptsIntersects() throws URISyntaxException {
final Path path = TestUtils.getPathOf("intersects_function.xml");
final FormValidator validator = new FormValidator();

Output output = Output.runAndGet(() -> validator.validate(path.toString()));
assertThat(output.getErr(), isEmptyString());
assertThat(output.getStd(), containsString("Xform is valid"));
}
}
48 changes: 48 additions & 0 deletions src/test/java/org/opendatakit/validate/Output.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.opendatakit.validate;

import java.io.ByteArrayOutputStream;
import java.io.PrintStream;

public class Output {
private final String std;
private final String err;

Output(String std, String err) {
this.std = std;
this.err = err;
}

static Output runAndGet(Runnable runnable) {
PrintStream outBackup = System.out;
ByteArrayOutputStream stdBaos = new ByteArrayOutputStream();
PrintStream stdPs = new PrintStream(stdBaos);
System.setOut(stdPs);

PrintStream errBackup = System.err;
ByteArrayOutputStream errBaos = new ByteArrayOutputStream();
PrintStream errPs = new PrintStream(errBaos);
System.setErr(errPs);

runnable.run();

stdPs.flush();
String std = stdBaos.toString();
System.setOut(outBackup);
System.out.print(std);

errPs.flush();
String err = errBaos.toString();
System.setErr(errBackup);
System.err.print(err);

return new Output(std, err);
}

public String getStd() {
return std;
}

public String getErr() {
return err;
}
}
11 changes: 11 additions & 0 deletions src/test/java/org/opendatakit/validate/TestUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package org.opendatakit.validate;

import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Paths;

public class TestUtils {
public static Path getPathOf(String filename) throws URISyntaxException {
return Paths.get(ValidateExternalSecondaryInstancesTest.class.getResource(filename.startsWith("/") ? filename : "/" + filename).toURI());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.isEmptyString;
import static org.opendatakit.validate.TestUtils.getPathOf;

import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Paths;
import org.junit.Test;

public class ValidateExternalSecondaryInstancesTest {
Expand All @@ -18,8 +16,8 @@ public void supportsXlsformsDefaultValueAndLabelRefs_inItemsets_usingXMLExternal
final FormValidator validator = new FormValidator();

Output output = Output.runAndGet(() -> validator.validate(path.toString()));
assertThat(output.err, isEmptyString());
assertThat(output.std, containsString("Xform is valid"));
assertThat(output.getErr(), isEmptyString());
assertThat(output.getStd(), containsString("Xform is valid"));
}

@Test
Expand All @@ -28,8 +26,8 @@ public void supportsXlsformsDefaultValueAndLabelRefs_inItemsets_usingCsvSecondar
final FormValidator validator = new FormValidator();

Output output = Output.runAndGet(() -> validator.validate(path.toString()));
assertThat(output.err, isEmptyString());
assertThat(output.std, containsString("Xform is valid"));
assertThat(output.getErr(), isEmptyString());
assertThat(output.getStd(), containsString("Xform is valid"));
}

@Test
Expand All @@ -38,48 +36,7 @@ public void supportsCustomValueAndLabelRefs_inItemsets_usingExternalSecondaryIns
final FormValidator validator = new FormValidator();

Output output = Output.runAndGet(() -> validator.validate(path.toString()));
assertThat(output.err, isEmptyString());
assertThat(output.std, containsString("Xform is valid"));
assertThat(output.getErr(), isEmptyString());
assertThat(output.getStd(), containsString("Xform is valid"));
}

private Path getPathOf(String filename) throws URISyntaxException {
return Paths.get(ValidateExternalSecondaryInstancesTest.class.getResource(filename.startsWith("/") ? filename : "/" + filename).toURI());
}

static class Output {
private final String std;
private final String err;

Output(String std, String err) {
this.std = std;
this.err = err;
}

static Output runAndGet(Runnable runnable) {
PrintStream outBackup = System.out;
ByteArrayOutputStream stdBaos = new ByteArrayOutputStream();
PrintStream stdPs = new PrintStream(stdBaos);
System.setOut(stdPs);

PrintStream errBackup = System.err;
ByteArrayOutputStream errBaos = new ByteArrayOutputStream();
PrintStream errPs = new PrintStream(errBaos);
System.setErr(errPs);

runnable.run();

stdPs.flush();
String std = stdBaos.toString();
System.setOut(outBackup);
System.out.print(std);

errPs.flush();
String err = errBaos.toString();
System.setErr(errBackup);
System.err.print(err);

return new Output(std, err);
}
}

}
20 changes: 20 additions & 0 deletions src/test/resources/intersects_function.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:odk="http://www.opendatakit.org/xforms"
xmlns:jr="http://openrosa.org/javarosa" xmlns:h="http://www.w3.org/1999/xhtml"
xmlns:orx="http://openrosa.org/xforms">
<h:head>
<h:title>Form with intersects</h:title>
<model>
<instance>
<data id="intersects">
<q1/>
<calc/>
</data>
</instance>
<bind nodeset="/data/calc" calculate="intersects(/data/q1)" type="string"/>
</model>
</h:head>
<h:body>
<input ref="/data/q1"/>
</h:body>
</h:html>