diff --git a/main/src/com/google/refine/InterProjectModel.java b/main/src/com/google/refine/InterProjectModel.java index 64a260b37..295042612 100644 --- a/main/src/com/google/refine/InterProjectModel.java +++ b/main/src/com/google/refine/InterProjectModel.java @@ -68,38 +68,22 @@ public class InterProjectModel { this.toProjectID = toProjectID; this.toProjectColumnName = toProjectColumnName; } - - public HasFieldsListImpl getRows(final Object rowKey, String separatorRegexp) { - Project toProject = ProjectManager.singleton.getProject(toProjectID); - if (toProject == null) { - return null; - } - - HasFieldsListImpl resultFieldList = null; - - if (ExpressionUtils.isNonBlankData(rowKey)) { - Object[] rowKeys; - if (separatorRegexp != null && !separatorRegexp.isEmpty() && rowKey instanceof String) { - rowKeys = ((String) rowKey).split(separatorRegexp); - } else { - rowKeys = new Object[]{rowKey}; - } - - resultFieldList = new HasFieldsListImpl(); - for (Object k : rowKeys) { - if (valueToRowIndices.containsKey(k)) { - for (Integer rowIndex : valueToRowIndices.get(k)) { - Row row = toProject.rows.get(rowIndex); - resultFieldList.add(new WrappedRow(toProject, rowIndex, row)); - } + + public HasFieldsListImpl getRows(Object value) { + if (ExpressionUtils.isNonBlankData(value) && valueToRowIndices.containsKey(value)) { + Project toProject = ProjectManager.singleton.getProject(toProjectID); + if (toProject != null) { + HasFieldsListImpl rows = new HasFieldsListImpl(); + for (Integer r : valueToRowIndices.get(value)) { + Row row = toProject.rows.get(r); + rows.add(new WrappedRow(toProject, r, row)); } + + return rows; } } - - // Returning null instead of an empty list is expected - return resultFieldList.isEmpty() ? null : resultFieldList; + return null; } - } protected Map _joins = new HashMap(); @@ -111,10 +95,9 @@ public class InterProjectModel { * @param fromColumn * @param toProject * @param toColumn - * @param separatorRegexp * @return */ - public ProjectJoin getJoin(String fromProject, String fromColumn, String toProject, String toColumn, String separatorRegexp) { + public ProjectJoin getJoin(String fromProject, String fromColumn, String toProject, String toColumn) { String key = fromProject + ";" + fromColumn + ";" + toProject + ";" + toColumn; if (!_joins.containsKey(key)) { ProjectJoin join = new ProjectJoin( @@ -123,8 +106,8 @@ public class InterProjectModel { ProjectManager.singleton.getProjectID(toProject), toColumn ); - - computeJoin(join, separatorRegexp); + + computeJoin(join); synchronized (_joins) { _joins.put(key, join); @@ -159,7 +142,7 @@ public class InterProjectModel { } } - protected void computeJoin(ProjectJoin join, String separatorRegexp) { + protected void computeJoin(ProjectJoin join) { if (join.fromProjectID < 0 || join.toProjectID < 0) { return; } @@ -175,21 +158,11 @@ public class InterProjectModel { if (fromColumn == null || toColumn == null) { return; } - + for (Row fromRow : fromProject.rows) { - Object fromRowKey = fromRow.getCellValue(fromColumn.getCellIndex()); - if (ExpressionUtils.isNonBlankData(fromRowKey)) { - Object[] fromRowKeys; - if (separatorRegexp != null && !separatorRegexp.isEmpty() && fromRowKey instanceof String) { - fromRowKeys = ((String) fromRowKey).split(separatorRegexp); - } else { - fromRowKeys = new Object[]{fromRowKey}; - } - for (Object k : fromRowKeys) { - if (!join.valueToRowIndices.containsKey(k)) { - join.valueToRowIndices.put(k, new ArrayList()); - } - } + Object value = fromRow.getCellValue(fromColumn.getCellIndex()); + if (ExpressionUtils.isNonBlankData(value) && !join.valueToRowIndices.containsKey(value)) { + join.valueToRowIndices.put(value, new ArrayList()); } } @@ -203,5 +176,4 @@ public class InterProjectModel { } } } - } diff --git a/main/src/com/google/refine/expr/functions/Cross.java b/main/src/com/google/refine/expr/functions/Cross.java index 883584365..93189212b 100644 --- a/main/src/com/google/refine/expr/functions/Cross.java +++ b/main/src/com/google/refine/expr/functions/Cross.java @@ -47,39 +47,33 @@ import com.google.refine.grel.Function; import com.google.refine.model.Project; public class Cross implements Function { - - public static final String EVAL_ERROR_MESSAGE = - " expects a string or cell, a project name to join with, and a column name in that project. " + - "Optional accepts a regular expression separator for source values."; - + @Override public Object call(Properties bindings, Object[] args) { - if (args.length >= 3) { + if (args.length == 3) { // 1st argument can take either value or cell(for backward compatibility) Object v = args[0]; Object toProjectName = args[1]; Object toColumnName = args[2]; - String separatorRegexp = (args.length > 3) ? String.valueOf(args[3]) : null; - - if (v != null && + + if (v != null && ( v instanceof String || v instanceof WrappedCell ) && toProjectName != null && toProjectName instanceof String && toColumnName != null && toColumnName instanceof String) { - + ProjectJoin join = ProjectManager.singleton.getInterProjectModel().getJoin( ProjectManager.singleton.getProjectMetadata(((Project) bindings.get("project")).id).getName(), (String) bindings.get("columnName"), - (String) toProjectName, - (String) toColumnName, - separatorRegexp - ); + (String) toProjectName, + (String) toColumnName + ); String srcValue = v instanceof String ? (String)v : (String)((WrappedCell) v).cell.value; - - return join.getRows(srcValue, separatorRegexp); + + return join.getRows(srcValue); } } - return new EvalError(ControlFunctionRegistry.getFunctionName(this) + EVAL_ERROR_MESSAGE); + return new EvalError(ControlFunctionRegistry.getFunctionName(this) + " expects a string or cell, a project name to join with, and a column name in that project"); } @Override @@ -88,7 +82,7 @@ public class Cross implements Function { writer.object(); writer.key("description"); writer.value("join with another project by column"); - writer.key("params"); writer.value("cell c or string value, string projectName, string columnName(, string separatorRegexp)"); + writer.key("params"); writer.value("cell c or string value, string projectName, string columnName"); writer.key("returns"); writer.value("array"); writer.endObject(); } diff --git a/main/tests/server/src/com/google/refine/tests/expr/functions/CrossFunctionTests.java b/main/tests/server/src/com/google/refine/tests/expr/functions/CrossFunctionTests.java index c22ba66d2..fbc93b5ff 100644 --- a/main/tests/server/src/com/google/refine/tests/expr/functions/CrossFunctionTests.java +++ b/main/tests/server/src/com/google/refine/tests/expr/functions/CrossFunctionTests.java @@ -9,7 +9,6 @@ import java.util.Calendar; import java.util.List; import java.util.Properties; -import com.google.refine.expr.functions.Cross; import org.json.JSONObject; import org.slf4j.LoggerFactory; import org.testng.Assert; @@ -164,34 +163,17 @@ public class CrossFunctionTests extends RefineTest { * rest of cells shows "Error: cross expects a string or cell, a project name to join with, and a column name in that project" */ @Test - public void crossFunctionIntegerValue() throws Exception { - String message = ((EvalError) invoke("cross", 1, "My Address Book", "friend")).message; - Assert.assertTrue(message.contains(Cross.EVAL_ERROR_MESSAGE), - String.format("Message should contain `%s` but is `%s`", Cross.EVAL_ERROR_MESSAGE, message)); + public void crossFunctionNonLiteralValue() throws Exception { + Assert.assertEquals(((EvalError) invoke("cross", 1, "My Address Book", "friend")).message, + "cross expects a string or cell, a project name to join with, and a column name in that project"); + + Assert.assertEquals(((EvalError) invoke("cross", null, "My Address Book", "friend")).message, + "cross expects a string or cell, a project name to join with, and a column name in that project"); + + Assert.assertEquals(((EvalError) invoke("cross", Calendar.getInstance(), "My Address Book", "friend")).message, + "cross expects a string or cell, a project name to join with, and a column name in that project"); } - - /** - * rest of cells shows "Error: cross expects a string or cell, a project name to join with, and a column name in - * that project" - */ - @Test - public void crossFunctionNull() throws Exception { - String message = ((EvalError) invoke("cross", null, "My Address Book", "friend")).message; - Assert.assertTrue(message.contains(Cross.EVAL_ERROR_MESSAGE), - String.format("Message should contain `%s` but is `%s`", Cross.EVAL_ERROR_MESSAGE, message)); - } - - /** - * rest of cells shows "Error: cross expects a string or cell, a project name to join with, and a column name in - * that project" - */ - @Test - public void crossFunctionCalendarInstance() throws Exception { - String message = ((EvalError) invoke("cross", Calendar.getInstance(), "My Address Book", "friend")).message; - Assert.assertTrue(message.contains(Cross.EVAL_ERROR_MESSAGE), - String.format("Message should contain `%s` but is `%s`", Cross.EVAL_ERROR_MESSAGE, message)); - } - + /** * Lookup a control function by name and invoke it with a variable number of args */