From a63151bbcc1b32f8dfc5a69de0ea9e5ab1fa7af8 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Mon, 4 Jun 2018 12:16:28 +0100 Subject: [PATCH] Do not discard the recon space when marking cells as new items. Fixes #1637 --- .../recon/ReconJudgeOneCellCommand.java | 15 ++- .../recon/ReconMarkNewTopicsOperation.java | 17 ++- .../recon/ReconJudgeOneCellCommandTest.java | 105 ++++++++++++++++++ 3 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 main/tests/server/src/com/google/refine/tests/commands/recon/ReconJudgeOneCellCommandTest.java diff --git a/main/src/com/google/refine/commands/recon/ReconJudgeOneCellCommand.java b/main/src/com/google/refine/commands/recon/ReconJudgeOneCellCommand.java index c9afb14f6..7297fdeea 100644 --- a/main/src/com/google/refine/commands/recon/ReconJudgeOneCellCommand.java +++ b/main/src/com/google/refine/commands/recon/ReconJudgeOneCellCommand.java @@ -167,10 +167,23 @@ 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 = diff --git a/main/src/com/google/refine/operations/recon/ReconMarkNewTopicsOperation.java b/main/src/com/google/refine/operations/recon/ReconMarkNewTopicsOperation.java index 161cac868..cccbdeb75 100644 --- a/main/src/com/google/refine/operations/recon/ReconMarkNewTopicsOperation.java +++ b/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 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; diff --git a/main/tests/server/src/com/google/refine/tests/commands/recon/ReconJudgeOneCellCommandTest.java b/main/tests/server/src/com/google/refine/tests/commands/recon/ReconJudgeOneCellCommandTest.java new file mode 100644 index 000000000..96cbaf7ac --- /dev/null +++ b/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); + } +}