From aa43445c994e5700c74acc6805d2ffaf2fd36cb5 Mon Sep 17 00:00:00 2001 From: Tom Morris Date: Sun, 30 Aug 2020 02:40:17 -0400 Subject: [PATCH] Extend forEach() to support JSON objects (#3150) * Refactor GREL Get tests - move helper up to RefineTest - move tests to the correct module * Extend forEach() to support JSON objects - fixes #3149 Also add tests for existing forEach forms in addition to the new one * Add a couple more tests --- .../google/refine/grel/controls/ForEach.java | 21 +++++- .../src/com/google/refine/RefineTest.java | 19 ++++++ .../refine/expr/functions/GetTests.java | 51 +++++++++++++- .../src/com/google/refine/grel/GrelTests.java | 21 ------ .../refine/grel/controls/ForEachTests.java | 67 ++++++++++++++++++- 5 files changed, 153 insertions(+), 26 deletions(-) diff --git a/main/src/com/google/refine/grel/controls/ForEach.java b/main/src/com/google/refine/grel/controls/ForEach.java index 0103d60d0..35243f3ab 100644 --- a/main/src/com/google/refine/grel/controls/ForEach.java +++ b/main/src/com/google/refine/grel/controls/ForEach.java @@ -38,7 +38,9 @@ import java.util.Collection; import java.util.List; import java.util.Properties; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.refine.expr.EvalError; import com.google.refine.expr.Evaluable; import com.google.refine.expr.ExpressionUtils; @@ -53,7 +55,7 @@ public class ForEach implements Control { if (args.length != 3) { return ControlFunctionRegistry.getControlName(this) + " expects 3 arguments"; } else if (!(args[1] instanceof VariableExpr)) { - return ControlFunctionRegistry.getControlName(this) + + return ControlFunctionRegistry.getControlName(this) + " expects second argument to be a variable name"; } return null; @@ -64,8 +66,9 @@ public class ForEach implements Control { Object o = args[0].evaluate(bindings); if (ExpressionUtils.isError(o)) { return o; - } else if (!ExpressionUtils.isArrayOrCollection(o) && !(o instanceof ArrayNode)) { - return new EvalError("First argument to forEach is not an array"); + } else if (!ExpressionUtils.isArrayOrCollection(o) && !(o instanceof ArrayNode) + && !(o instanceof ObjectNode)) { + return new EvalError("First argument to forEach is not an array or JSON object"); } String name = ((VariableExpr) args[1]).getName(); @@ -105,6 +108,18 @@ public class ForEach implements Control { Object r = args[2].evaluate(bindings); + results.add(r); + } + } else if (o instanceof ObjectNode) { + ObjectNode obj = (ObjectNode) o; + results = new ArrayList(obj.size()); + for (JsonNode v : obj) { + if (v != null) { + bindings.put(name, v); + } else { + bindings.remove(name); + } + Object r = args[2].evaluate(bindings); results.add(r); } } else { diff --git a/main/tests/server/src/com/google/refine/RefineTest.java b/main/tests/server/src/com/google/refine/RefineTest.java index 22b29ec6c..b7cbb6374 100644 --- a/main/tests/server/src/com/google/refine/RefineTest.java +++ b/main/tests/server/src/com/google/refine/RefineTest.java @@ -58,6 +58,9 @@ import com.fasterxml.jackson.databind.node.BooleanNode; import com.fasterxml.jackson.databind.node.IntNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.TextNode; +import com.google.refine.expr.Evaluable; +import com.google.refine.expr.MetaParser; +import com.google.refine.expr.ParsingException; import com.google.refine.grel.ControlFunctionRegistry; import com.google.refine.grel.Function; import com.google.refine.importers.SeparatorBasedImporter; @@ -332,6 +335,22 @@ public class RefineTest extends PowerMockTestCase { } } + + /** + * Parse and evaluate a GREL expression and compare the result to the expect value + * + * @param bindings + * @param test + * @throws ParsingException + */ + protected void parseEval(Properties bindings, String[] test) + throws ParsingException { + Evaluable eval = MetaParser.parse("grel:" + test[0]); + Object result = eval.evaluate(bindings); + Assert.assertEquals(result.toString(), test[1], "Wrong result for expression: " + test[0]); + } + + @AfterMethod public void TearDown() throws Exception { bindings = null; diff --git a/main/tests/server/src/com/google/refine/expr/functions/GetTests.java b/main/tests/server/src/com/google/refine/expr/functions/GetTests.java index 2c009a4d8..b9f515df8 100644 --- a/main/tests/server/src/com/google/refine/expr/functions/GetTests.java +++ b/main/tests/server/src/com/google/refine/expr/functions/GetTests.java @@ -26,15 +26,64 @@ ******************************************************************************/ package com.google.refine.expr.functions; +import java.util.Properties; + +import org.slf4j.LoggerFactory; +import org.testng.Assert; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; +import com.google.refine.RefineTest; +import com.google.refine.expr.Evaluable; +import com.google.refine.expr.ExpressionUtils; +import com.google.refine.expr.MetaParser; +import com.google.refine.expr.ParsingException; +import com.google.refine.model.Project; import com.google.refine.util.TestUtils; -public class GetTests { +public class GetTests extends RefineTest { + + Project project; + Properties bindings; + + @Override + @BeforeTest + public void init() { + logger = LoggerFactory.getLogger(this.getClass()); + } + + @BeforeMethod + public void SetUp() { + project = new Project(); + bindings = ExpressionUtils.createBindings(project); + } + + @AfterMethod + public void TearDown() { + project = null; + bindings = null; + } + @Test public void serializeGet() { String json = "{\"description\":\"If o has fields, returns the field named 'from' of o. If o is an array, returns o[from, to]. if o is a string, returns o.substring(from, to)\",\"params\":\"o, number or string from, optional number to\",\"returns\":\"Depends on actual arguments\"}"; TestUtils.isSerializedTo(new Get(), json); } + + @Test + public void testGetJsonFieldExists() throws ParsingException { + String test[] = { "\"[{\\\"one\\\": \\\"1\\\"}]\".parseJson()[0].one", "1" }; + parseEval(bindings, test); + } + + @Test + public void testGetJsonFieldAbsent() throws ParsingException { + String test = "\"[{\\\"one\\\": \\\"1\\\"}]\".parseJson()[0].two"; + Evaluable eval = MetaParser.parse("grel:" + test); + Assert.assertNull(eval.evaluate(bindings)); + } + } diff --git a/main/tests/server/src/com/google/refine/grel/GrelTests.java b/main/tests/server/src/com/google/refine/grel/GrelTests.java index e332075aa..d59c9b667 100644 --- a/main/tests/server/src/com/google/refine/grel/GrelTests.java +++ b/main/tests/server/src/com/google/refine/grel/GrelTests.java @@ -167,19 +167,6 @@ public class GrelTests extends RefineTest { } } - @Test - public void testGetJsonFieldExists() throws ParsingException { - String test[] = { "\"[{\\\"one\\\": \\\"1\\\"}]\".parseJson()[0].one", "1" }; - parseEval(bindings, test); - } - - @Test - public void testGetJsonFieldAbsent() throws ParsingException { - String test = "\"[{\\\"one\\\": \\\"1\\\"}]\".parseJson()[0].two"; - Evaluable eval = MetaParser.parse("grel:" + test); - Assert.assertNull(eval.evaluate(bindings)); - } - @Test public void testJoinJsonArray() throws ParsingException { String test[] = { "\"{\\\"values\\\":[\\\"one\\\",\\\"two\\\",\\\"three\\\"]}\".parseJson().values.join(\",\")", "one,two,three" }; @@ -206,13 +193,5 @@ public class GrelTests extends RefineTest { Assert.fail("Unexpected parse failure for cross function: " + test); } } - - private void parseEval(Properties bindings, String[] test) - throws ParsingException { - Evaluable eval = MetaParser.parse("grel:" + test[0]); - Object result = eval.evaluate(bindings); - Assert.assertEquals(result.toString(), test[1], - "Wrong result for expression: "+test[0]); - } } diff --git a/main/tests/server/src/com/google/refine/grel/controls/ForEachTests.java b/main/tests/server/src/com/google/refine/grel/controls/ForEachTests.java index ec1e30e6c..2c2aac1a3 100644 --- a/main/tests/server/src/com/google/refine/grel/controls/ForEachTests.java +++ b/main/tests/server/src/com/google/refine/grel/controls/ForEachTests.java @@ -26,15 +26,80 @@ ******************************************************************************/ package com.google.refine.grel.controls; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; + +import java.util.Properties; + import org.testng.annotations.Test; +import com.google.refine.RefineTest; +import com.google.refine.expr.EvalError; +import com.google.refine.expr.Evaluable; +import com.google.refine.expr.MetaParser; +import com.google.refine.expr.ParsingException; import com.google.refine.util.TestUtils; -public class ForEachTests { +public class ForEachTests extends RefineTest { @Test public void serializeForEach() { String json = "{\"description\":\"Evaluates expression a to an array. Then for each array element, binds its value to variable name v, evaluates expression e, and pushes the result onto the result array.\",\"params\":\"expression a, variable v, expression e\",\"returns\":\"array\"}"; TestUtils.isSerializedTo(new ForEach(), json); } + + private void assertParseError(String expression) throws ParsingException { + Evaluable eval = MetaParser.parse("grel:" + expression); + Object result = eval.evaluate(bindings); + assertTrue(result instanceof EvalError, "Expression didn't return error : " + expression); + } + + @Test + public void testInvalidParams() throws ParsingException { + bindings = new Properties(); + bindings.put("v", ""); + assertParseError("forEach('test', v, v)"); + try { + assertParseError("forEach([], 1, 1)"); + fail("Didn't throw a ParsingException for wrong argument type"); + } catch (ParsingException e) {} + try { + assertParseError("forEach([], v)"); + fail("Didn't throw a ParsingException for 2 arguments"); + } catch (ParsingException e) {} + try { + assertParseError("forEach([])"); + fail("Didn't throw a ParsingException for 1 argument"); + } catch (ParsingException e) {} + } + + @Test + public void testForEachJsonObject() throws ParsingException { + String json = "{" + + "\"2511\": {\"parent_area\": null, \"generation_high\": 40, \"all_names\": {}, \"id\": 2511, \"codes\": {\"ons\": \"00AB\", \"gss\": \"E09000002\", \"local-authority-eng\": \"BDG\", \"local-authority-canonical\": \"BDG\", \"unit_id\": \"10949\"}, \"name\": \"Barking and Dagenham Borough Council\", \"country\": \"E\", \"type_name\": \"London borough\", \"generation_low\": 1, \"country_name\": \"England\", \"type\": \"LBO\"}," + + "\"2247\": {\"parent_area\": null, \"generation_high\": 40, \"all_names\": {}, \"id\": 2247, \"codes\": {\"unit_id\": \"41441\"}, \"name\": \"Greater London Authority\", \"country\": \"E\", \"type_name\": \"Greater London Authority\", \"generation_low\": 1, \"country_name\": \"England\", \"type\": \"GLA\"}," + + "\"8706\": {\"parent_area\": 2511, \"generation_high\": 40, \"all_names\": {}, \"id\": 8706, \"codes\": {\"ons\": \"00ABGH\", \"gss\": \"E05000036\", \"unit_id\": \"10936\"}, \"name\": \"Mayesbrook\", \"country\": \"E\", \"type_name\": \"London borough ward\", \"generation_low\": 1, \"country_name\": \"England\", \"type\": \"LBW\"}" + + "}"; + + String test[] = {"forEach('" + json + "'.parseJson(), v, v.id).sort().join(',')", "2247,2511,8706"}; + bindings = new Properties(); + bindings.put("v", ""); + parseEval(bindings, test); + } + + @Test + public void testForEachArray() throws ParsingException { + String test[] = {"forEach([5,4,3,2.0], v, v*2).join(',')", "10,8,6,4.0"}; + bindings = new Properties(); + bindings.put("v", ""); + parseEval(bindings, test); + } + + @Test + public void testForEachJsonArray() throws ParsingException { + String test[] = {"forEach('[3,2,1.0]'.parseJson(), v, v*2).join(',')", "6,4,2.0"}; + bindings = new Properties(); + bindings.put("v", ""); + parseEval(bindings, test); + } }