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.

Sign up for GitHub

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 multi-argument and single string distance variants #761

Merged
merged 3 commits into from
May 21, 2024
Merged

Add multi-argument and single string distance variants #761

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f04d128
Add multi-argument distance variant
lognaturel May 10, 2024
fb3557b
Add support for single string argument
lognaturel May 14, 2024
4d71a59
Separate out test for mixed argument types
lognaturel May 21, 2024
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
45 changes: 28 additions & 17 deletions src/main/java/org/javarosa/xpath/expr/XPathFuncExpr.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@

package org.javarosa.xpath.expr;

import static java.lang.Double.NaN;

import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import java.math.BigDecimal;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.regex.Pattern;
import org.javarosa.core.model.condition.EvaluationContext;
import org.javarosa.core.model.condition.IFallbackFunctionHandler;
import org.javarosa.core.model.condition.IFunctionHandler;
Expand Down Expand Up @@ -43,19 +55,6 @@
import org.jetbrains.annotations.NotNull;
import org.joda.time.DateTime;

import java.io.DataInputStream;
import java.io.DataOutputStream;
import java.io.IOException;
import java.math.BigDecimal;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.regex.Pattern;

import static java.lang.Double.NaN;

/**
* Representation of an xpath function expression.
* <p>
Expand Down Expand Up @@ -476,9 +475,21 @@ public Object eval(DataInstance model, EvaluationContext evalContext) {
List<GeoUtils.LatLong> latLongs = new XPathFuncExprGeo().getGpsCoordinatesFromNodeset(name, argVals[0]);
return GeoUtils.calculateAreaOfGPSPolygonOnEarthInSquareMeters(latLongs);
} else if (name.equals("distance")) {
assertArgsCount(name, args, 1);
List<GeoUtils.LatLong> latLongs = new XPathFuncExprGeo().getGpsCoordinatesFromNodeset(name, argVals[0]);
return GeoUtils.calculateDistance(latLongs);
if (args.length == 1) {
if (argVals[0] instanceof XPathNodeset) {
List<GeoUtils.LatLong> latLongs = new XPathFuncExprGeo().getGpsCoordinatesFromNodeset(name, argVals[0]);
return GeoUtils.calculateDistance(latLongs);
} else if (argVals[0] instanceof String) {
List<GeoUtils.LatLong> latLongs = new XPathFuncExprGeo().geopointsToLatLongs(name, ((String) argVals[0]).split(";"));
return GeoUtils.calculateDistance(latLongs);
} else {
throw new XPathUnhandledException("function 'distance' requires a field or text as the parameter.");
}
} else if (args.length > 1) {
return GeoUtils.calculateDistance(new XPathFuncExprGeo().geopointsToLatLongs(name, argVals));
} else {
throw new XPathUnhandledException("function 'distance' requires at least one parameter.");
}
} else if (name.equals("digest") && (args.length == 2 || args.length == 3)) {
return DigestAlgorithm.from(toString(argVals[1])).digest(
toString(argVals[0]),
Expand All @@ -494,7 +505,7 @@ public Object eval(DataInstance model, EvaluationContext evalContext) {
if (args.length == 2)
return XPathNodeset.shuffle((XPathNodeset) argVals[0], toNumeric(argVals[1]).longValue());

throw new XPathUnhandledException("function \'randomize\' requires 1 or 2 arguments. " + args.length + " provided.");
throw new XPathUnhandledException("function 'randomize' requires 1 or 2 arguments. " + args.length + " provided.");
} else if (name.equals("base64-decode")) {
assertArgsCount(name, args, 1);
return base64Decode(argVals[0]);
Expand Down
30 changes: 17 additions & 13 deletions src/main/java/org/javarosa/xpath/expr/XPathFuncExprGeo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.javarosa.xpath.expr;

import java.util.ArrayList;
import java.util.List;
import org.javarosa.core.model.data.GeoPointData;
import org.javarosa.core.model.data.GeoShapeData;
import org.javarosa.core.model.data.UncastData;
Expand All @@ -10,16 +12,13 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

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

/** XPath function expression geographic logic */
class XPathFuncExprGeo {
private static final Logger logger = LoggerFactory.getLogger(XPathFuncExprGeo.class.getSimpleName());

List<GeoUtils.LatLong> getGpsCoordinatesFromNodeset(String name, Object argVal) {
if (!(argVal instanceof XPathNodeset)) {
throw new XPathUnhandledException("function \'" + name + "\' requires a field as the parameter.");
throw new XPathUnhandledException("function '" + name + "' requires a field as the parameter.");
}
Object[] argList = ((XPathNodeset) argVal).toArgList();
int repeatSize = argList.length;
Expand All @@ -37,17 +36,22 @@ List<GeoUtils.LatLong> getGpsCoordinatesFromNodeset(String name, Object argVal)
throwMismatch(name);
}
} else if (repeatSize >= 2) {
// treat the input as a series of GeoPointData

for (Object arg : argList) {
try {
GeoPointData geoPointData = new GeoPointData().cast(new UncastData(XPathFuncExpr.toString(arg)));
latLongs.add(new GeoUtils.LatLong(geoPointData.getPart(0), geoPointData.getPart(1)));
} catch (Exception e) {
throwMismatch(name);
}
latLongs.addAll(geopointsToLatLongs(name, argList));
}
return latLongs;
}

public List<GeoUtils.LatLong> geopointsToLatLongs(String callingFunction, Object[] args) {
List<GeoUtils.LatLong> latLongs = new ArrayList<>();
for (Object arg : args) {
try {
GeoPointData geoPointData = new GeoPointData().cast(new UncastData(XPathFuncExpr.toString(arg)));
latLongs.add(new GeoUtils.LatLong(geoPointData.getPart(0), geoPointData.getPart(1)));
} catch (Exception e) {
throwMismatch(callingFunction);
}
}

return latLongs;
}

Expand Down
90 changes: 89 additions & 1 deletion src/test/java/org/javarosa/core/util/GeoDistanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@

package org.javarosa.core.util;

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.closeTo;
import static org.javarosa.core.util.BindBuilderXFormsElement.bind;
import static org.javarosa.core.util.GeoUtils.EARTH_EQUATORIAL_CIRCUMFERENCE_METERS;
Expand All @@ -29,12 +32,13 @@
import static org.javarosa.core.util.XFormsElement.repeat;
import static org.javarosa.core.util.XFormsElement.t;
import static org.javarosa.core.util.XFormsElement.title;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

import java.io.IOException;
import org.hamcrest.number.IsCloseTo;
import org.javarosa.core.test.Scenario;
import org.javarosa.xform.parse.XFormParser;
import org.javarosa.xpath.XPathTypeMismatchException;
import org.junit.Test;

public class GeoDistanceTest {
Expand Down Expand Up @@ -149,6 +153,90 @@ public void distance_isComputedForString() throws IOException, XFormParser.Parse
IsCloseTo.closeTo(1801, 0.5));
}

@Test
public void distance_isComputedForInlineString() throws IOException, XFormParser.ParseException {
Scenario scenario = Scenario.init("string distance", html(
head(
title("String distance"),
model(
mainInstance(t("data id=\"string-distance\"",
t("point1", "38.253094215699576 21.756382658677467 0 0"),
t("point2", "38.25021274773806 21.756382658677467 0 0"),
t("point3", "38.25007793942195 21.763892843919166 0 0"),
t("point4", "38.25290886154963 21.763935759263404 0 0"),
t("point5", "38.25146813817506 21.758421137528785 0 0"),
t("distance")
)),
bind("/data/point1").type("geopoint"),
bind("/data/point2").type("geopoint"),
bind("/data/point3").type("geopoint"),
bind("/data/point4").type("geopoint"),
bind("/data/point5").type("geopoint"),
bind("/data/distance").type("decimal").calculate("distance(concat(/data/point1, ';', /data/point2, ';', /data/point3, ';', /data/point4, ';', /data/point5))")
)),
body(
input("/data/point1")
)
));

// http://www.mapdevelopers.com/area_finder.php?&points=%5B%5B38.253094215699576%2C21.756382658677467%5D%2C%5B38.25021274773806%2C21.756382658677467%5D%2C%5B38.25007793942195%2C21.763892843919166%5D%2C%5B38.25290886154963%2C21.763935759263404%5D%2C%5B38.25146813817506%2C21.758421137528785%5D%5D
assertThat(Double.parseDouble(scenario.answerOf("/data/distance").getDisplayText()),
IsCloseTo.closeTo(1801, 0.5));
}

@Test
public void distance_throwsForNonPoint() throws IOException, XFormParser.ParseException {
try {
Scenario.init("string distance", html(
head(
title("String distance"),
model(
mainInstance(t("data id=\"string-distance\"",
t("distance")
)),
bind("/data/distance").type("decimal").calculate("distance('foo')")
)),
body(
input("distance")
)
));

fail("Exception expected");
} catch (RuntimeException e) {
assertThat(e.getCause(), instanceOf(XPathTypeMismatchException.class));
assertThat(e.getMessage(), containsString("The function 'distance' received a value that does not represent GPS coordinates"));
}
}

@Test
public void distance_isComputedForMultipleArguments() throws IOException, XFormParser.ParseException {
Scenario scenario = Scenario.init("string distance", html(
head(
title("Multi parameter distance"),
model(
mainInstance(t("data id=\"string-distance\"",
t("point1", "38.253094215699576 21.756382658677467 0 0"),
t("point3", "38.25007793942195 21.763892843919166 0 0"),
t("point4", "38.25290886154963 21.763935759263404 0 0"),
t("point5", "38.25146813817506 21.758421137528785 0 0"),
t("distance")
)),
bind("/data/point1").type("geopoint"),
bind("/data/point3").type("geopoint"),
bind("/data/point4").type("geopoint"),
bind("/data/point5").type("geopoint"),
bind("/data/distance").type("decimal").calculate("distance(/data/point1, '38.25021274773806 21.756382658677467 0 0', /data/point3, /data/point4, /data/point5)")
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing "point 2" is not pulled from a node to test that literals work fine here? I think if that's a concern it should move out to a separate test (or maybe a unit test of XPathFuncExpr) so it's not hidden in here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Allowing a mix of references and literals is an explicit requirement so I wanted to make sure it was tested. Maybe I split it out into a Scenario test with only references and one with a mix? We don't currently have unit tests that include type -- that ends up being more of an integration concern.

)),
body(
input("/data/point1")
)
));

// http://www.mapdevelopers.com/area_finder.php?&points=%5B%5B38.253094215699576%2C21.756382658677467%5D%2C%5B38.25021274773806%2C21.756382658677467%5D%2C%5B38.25007793942195%2C21.763892843919166%5D%2C%5B38.25290886154963%2C21.763935759263404%5D%2C%5B38.25146813817506%2C21.758421137528785%5D%5D
assertThat(Double.parseDouble(scenario.answerOf("/data/distance").getDisplayText()),
IsCloseTo.closeTo(1801, 0.5));
}

@Test
public void distance_whenTraceHasFewerThanTwoPoints_isZero() throws Exception {
Scenario scenario = Scenario.init("geotrace distance", html(
Expand Down