Merge pull request #1291 from OpenRefine/revert-1290-cross-func-split

Revert "Extend cross() function to support multiple-value-cell-input"

main/src/com/google/refine/InterProjectModel.java

@@ -69,37 +69,21 @@ public class InterProjectModel {
             this.toProjectColumnName = toProjectColumnName;
         }
         
-        public HasFieldsListImpl getRows(final Object rowKey, String separatorRegexp) {
+        public HasFieldsListImpl getRows(Object value) {
+            if (ExpressionUtils.isNonBlankData(value) && valueToRowIndices.containsKey(value)) {
                 Project toProject = ProjectManager.singleton.getProject(toProjectID);
-            if (toProject == null) {
+                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;
+                }
+            }
             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));
-                        }
-                    }
-                }
-            }
-
-            // Returning null instead of an empty list is expected
-            return resultFieldList.isEmpty() ? null : resultFieldList;
-        }
-
     }
     
     protected Map<String, ProjectJoin> _joins = new HashMap<String, ProjectJoin>();
@@ -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(
@@ -124,7 +107,7 @@ public class InterProjectModel {
                 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;
         }
@@ -177,19 +160,9 @@ public class InterProjectModel {
         }
         
         for (Row fromRow : fromProject.rows) {
-            Object fromRowKey = fromRow.getCellValue(fromColumn.getCellIndex());
+            Object value = fromRow.getCellValue(fromColumn.getCellIndex());
-            if (ExpressionUtils.isNonBlankData(fromRowKey)) {
+            if (ExpressionUtils.isNonBlankData(value) && !join.valueToRowIndices.containsKey(value)) {
-                Object[] fromRowKeys;
+                join.valueToRowIndices.put(value, new ArrayList<Integer>());
-                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<Integer>());
-                    }
-                }
             }
         }
         
@@ -203,5 +176,4 @@ public class InterProjectModel {
             }
         }
     }
-
 }

main/src/com/google/refine/expr/functions/Cross.java

@@ -48,18 +48,13 @@ 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 && 
                 ( v instanceof String || v instanceof WrappedCell ) &&
@@ -70,16 +65,15 @@ public class Cross implements Function {
                         ProjectManager.singleton.getProjectMetadata(((Project) bindings.get("project")).id).getName(),
                         (String) bindings.get("columnName"), 
                         (String) toProjectName, 
-                        (String) toColumnName,
+                        (String) toColumnName
-                        separatorRegexp
                         );
                 
                 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();
     }

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,32 +163,15 @@ 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 {
+    public void crossFunctionNonLiteralValue() throws Exception {
-        String message = ((EvalError) invoke("cross", 1, "My Address Book", "friend")).message;
+        Assert.assertEquals(((EvalError) invoke("cross", 1, "My Address Book", "friend")).message, 
-        Assert.assertTrue(message.contains(Cross.EVAL_ERROR_MESSAGE),
+                "cross expects a string or cell, a project name to join with, and a column name in that project");
-                String.format("Message should contain `%s` but is `%s`", Cross.EVAL_ERROR_MESSAGE, message));
-    }
         
-    /**
+        Assert.assertEquals(((EvalError) invoke("cross", null, "My Address Book", "friend")).message, 
-     * rest of cells shows "Error: cross expects a string or cell, a project name to join with, and a column name in
+                "cross expects a string or cell, a project name to join with, and a column name in that project");
-     * 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));
-    }
         
-    /**
+        Assert.assertEquals(((EvalError) invoke("cross", Calendar.getInstance(), "My Address Book", "friend")).message, 
-     * rest of cells shows "Error: cross expects a string or cell, a project name to join with, and a column name in
+                "cross expects a string or cell, a project name to join with, and a column name in that project");
-     * 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));
     }
     
     /**