Merge pull request #1638 from OpenRefine/issue1637

Do not discard the recon space when marking cells as new items.

main/src/com/google/refine/commands/recon/ReconJudgeOneCellCommand.java

@@ -168,9 +168,22 @@ public class ReconJudgeOneCellCommand extends Command {
 
             Judgment oldJudgment = cell.recon == null ? Judgment.None : cell.recon.judgment;
             
+            Recon newRecon = null;
+            if (cell.recon != null) {
+                newRecon = cell.recon.dup(historyEntryID);
+            } else if (identifierSpace != null && schemaSpace != null) {
+                newRecon = new Recon(historyEntryID, identifierSpace, schemaSpace);
+            } else if (column.getReconConfig() != null) {
+                newRecon = column.getReconConfig().createNewRecon(historyEntryID);
+            } else {
+                // This should only happen if we are judging a cell in a column that
+                // has never been reconciled before.
+               newRecon = new Recon(historyEntryID, null, null);
+            }
+
             newCell = new Cell(
                 cell.value,
-                cell.recon == null ? new Recon(historyEntryID, identifierSpace, schemaSpace) : cell.recon.dup(historyEntryID)
+                newRecon
             );
 
             String cellDescription =

main/src/com/google/refine/operations/recon/ReconMarkNewTopicsOperation.java

@@ -53,6 +53,7 @@ import com.google.refine.model.Recon.Judgment;
 import com.google.refine.model.Row;
 import com.google.refine.model.changes.CellChange;
 import com.google.refine.model.changes.ReconChange;
+import com.google.refine.model.recon.ReconConfig;
 import com.google.refine.operations.EngineDependentMassCellOperation;
 import com.google.refine.operations.OperationRegistry;
 
@@ -109,6 +110,7 @@ public class ReconMarkNewTopicsOperation extends EngineDependentMassCellOperatio
     @Override
     protected RowVisitor createRowVisitor(Project project, List<CellChange> cellChanges, long historyEntryID) throws Exception {
         Column column = project.columnModel.getColumnByName(_columnName);
+        ReconConfig reconConfig = column.getReconConfig();
         
         return new RowVisitor() {
             int                 cellIndex;
@@ -133,6 +135,17 @@ public class ReconMarkNewTopicsOperation extends EngineDependentMassCellOperatio
                 // nothing to do
             }
             
+            private Recon createNewRecon() {
+                if(reconConfig != null) {
+                    return reconConfig.createNewRecon(historyEntryID);
+                } else {
+                    // This should only happen when marking cells as reconciled
+                    // in a column that has never been reconciled before. In this case,
+                    // we just resort to the default reconciliation space.
+                    return new Recon(historyEntryID, null, null);
+                }
+            }
+            
             @Override
             public boolean visit(Project project, int rowIndex, Row row) {
                 Cell cell = row.getCell(cellIndex);
@@ -144,7 +157,7 @@ public class ReconMarkNewTopicsOperation extends EngineDependentMassCellOperatio
                             recon = sharedRecons.get(s);
                             recon.judgmentBatchSize++;
                         } else {
-                            recon = new Recon(historyEntryID, null, null);
+                            recon = createNewRecon();
                             recon.judgment = Judgment.New;
                             recon.judgmentBatchSize = 1;
                             recon.judgmentAction = "mass";
@@ -152,7 +165,7 @@ public class ReconMarkNewTopicsOperation extends EngineDependentMassCellOperatio
                             sharedRecons.put(s, recon);
                         }
                     } else {
-                        recon = cell.recon == null ? new Recon(historyEntryID, null, null) : cell.recon.dup(historyEntryID);
+                        recon = cell.recon == null ? createNewRecon() : cell.recon.dup(historyEntryID);
                         recon.match = null;
                         recon.matchRank = -1;
                         recon.judgment = Judgment.New;

main/tests/server/src/com/google/refine/tests/commands/recon/ReconJudgeOneCellCommandTest.java

@@ -0,0 +1,105 @@
+package com.google.refine.tests.commands.recon;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.util.Collections;
+import java.util.Properties;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.json.JSONObject;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.google.refine.ProjectManager;
+import com.google.refine.commands.Command;
+import com.google.refine.commands.recon.ReconJudgeOneCellCommand;
+import com.google.refine.model.Cell;
+import com.google.refine.model.Column;
+import com.google.refine.model.Project;
+import com.google.refine.model.Recon;
+import com.google.refine.model.recon.ReconConfig;
+import com.google.refine.model.recon.StandardReconConfig;
+import com.google.refine.tests.RefineTest;
+
+public class ReconJudgeOneCellCommandTest extends RefineTest {
+        
+    Project project = null;
+    HttpServletRequest request = null;
+    HttpServletResponse response = null;
+    Command command = null;
+    PrintWriter writer = null;
+    
+    @BeforeMethod
+    public void setUp() {
+        project = createCSVProject(
+                "reconciled column,unreconciled column\n"+
+                "a,b\n"+
+                "c,d\n");
+        Column reconciled = project.columnModel.columns.get(0);
+        ReconConfig config = new StandardReconConfig(
+                "http://my.recon.service/api",
+                "http://my.recon.service/rdf/space",
+                "http://my.recon.service/rdf/schema",
+                "type3894", 
+                "octopus",
+                true,
+                Collections.emptyList(),
+                5);
+        reconciled.setReconConfig(config);
+        
+        request = mock(HttpServletRequest.class);
+        response = mock(HttpServletResponse.class);
+        
+        when(request.getParameter("project")).thenReturn(String.valueOf(project.id));
+        
+        writer = mock(PrintWriter.class);
+        try {
+            when(response.getWriter()).thenReturn(writer);
+        } catch (IOException e1) {
+            Assert.fail();
+        }
+        
+        command = new  ReconJudgeOneCellCommand();
+    }
+    
+    @AfterMethod
+    public void tearDown() {
+        ProjectManager.singleton.deleteProject(project.id);
+    }
+    
+    @Test
+    public void testMarkOneCellInReconciledColumn() throws Exception {
+
+        when(request.getParameter("row")).thenReturn("0");
+        when(request.getParameter("cell")).thenReturn("0");
+        when(request.getParameter("judgment")).thenReturn("new");
+        command.doPost(request, response);
+        
+        Cell cell = project.rows.get(0).cells.get(0);
+        Assert.assertEquals(Recon.Judgment.New, cell.recon.judgment);
+        Assert.assertEquals("http://my.recon.service/rdf/space", cell.recon.identifierSpace);
+    }
+    
+    @Test
+    public void testMarkOneCellWithCustomSpace() throws Exception {
+
+        when(request.getParameter("row")).thenReturn("0");
+        when(request.getParameter("cell")).thenReturn("0");
+        when(request.getParameter("judgment")).thenReturn("new");
+        when(request.getParameter("identifierSpace")).thenReturn("http://my.custom.space/id");
+        when(request.getParameter("schemaSpace")).thenReturn("http://my.custom.space/schema");
+        command.doPost(request, response);
+        
+        Cell cell = project.rows.get(0).cells.get(0);
+        Assert.assertEquals(Recon.Judgment.New, cell.recon.judgment);
+        Assert.assertEquals("http://my.custom.space/id", cell.recon.identifierSpace);
+        Assert.assertEquals("http://my.custom.space/schema", cell.recon.schemaSpace);
+    }
+}