From 6e4dfe67af0a1459eebf3510c9726e67340601ec Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Mon, 19 Mar 2018 08:57:40 +0000 Subject: [PATCH] Fix bug conflating new items --- .../openrefine/wikidata/editing/NewItemLibrary.java | 6 +++--- .../schema/entityvalues/ReconEntityIdValue.java | 4 ++-- .../wikidata/editing/NewItemLibraryTest.java | 2 +- .../exporters/QuickStatementsExporterTest.java | 4 ++-- .../schema/entityvalues/ReconEntityIdValueTest.java | 4 ++-- .../org/openrefine/wikidata/testing/TestingData.java | 12 ++++++++++-- .../updates/scheduler/UpdateSchedulerTest.java | 4 ++-- 7 files changed, 22 insertions(+), 14 deletions(-) diff --git a/extensions/wikidata/src/org/openrefine/wikidata/editing/NewItemLibrary.java b/extensions/wikidata/src/org/openrefine/wikidata/editing/NewItemLibrary.java index c76a7d312..b84af31cb 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/editing/NewItemLibrary.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/editing/NewItemLibrary.java @@ -108,13 +108,13 @@ public class NewItemLibrary { } Recon recon = cell.recon; if (Recon.Judgment.New.equals(recon.judgment) && !reset - && map.containsKey(recon.judgmentHistoryEntry)) { + && map.containsKey(recon.id)) { recon.judgment = Recon.Judgment.Matched; - recon.match = new ReconCandidate(map.get(recon.judgmentHistoryEntry), cell.value.toString(), + recon.match = new ReconCandidate(map.get(recon.id), cell.value.toString(), new String[0], 100); impactedColumns.add(i); } else if (Recon.Judgment.Matched.equals(recon.judgment) && reset - && map.containsKey(recon.judgmentHistoryEntry)) { + && map.containsKey(recon.id)) { recon.judgment = Recon.Judgment.New; recon.match = null; impactedColumns.add(i); diff --git a/extensions/wikidata/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValue.java b/extensions/wikidata/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValue.java index d9b4f6ee9..2ab8f3222 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValue.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValue.java @@ -94,10 +94,10 @@ public abstract class ReconEntityIdValue implements PrefetchedEntityIdValue { /** * Returns the integer used internally in OpenRefine to identify the new item. * - * @return the judgment history entry id of the reconciled cell + * @return the reconciliation id of the reconciled cell */ public long getReconInternalId() { - return getRecon().judgmentHistoryEntry; + return getRecon().id; } /** diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/editing/NewItemLibraryTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/editing/NewItemLibraryTest.java index 193dbf32f..3cb2063a3 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/editing/NewItemLibraryTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/editing/NewItemLibraryTest.java @@ -83,6 +83,6 @@ public class NewItemLibraryTest extends RefineTest { private void isNewTo(long id, Cell cell) { assertEquals(Recon.Judgment.New, cell.recon.judgment); - assertEquals(id, cell.recon.judgmentHistoryEntry); + assertEquals(id, cell.recon.id); } } diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/exporters/QuickStatementsExporterTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/exporters/QuickStatementsExporterTest.java index cc3e2766e..dfc15bc9d 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/exporters/QuickStatementsExporterTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/exporters/QuickStatementsExporterTest.java @@ -53,8 +53,8 @@ import com.google.refine.tests.RefineTest; public class QuickStatementsExporterTest extends RefineTest { private QuickStatementsExporter exporter = new QuickStatementsExporter(); - private ItemIdValue newIdA = TestingData.makeNewItemIdValue(1234L, "new item A"); - private ItemIdValue newIdB = TestingData.makeNewItemIdValue(5678L, "new item B"); + private ItemIdValue newIdA = TestingData.newIdA; + private ItemIdValue newIdB = TestingData.newIdB; private ItemIdValue qid1 = Datamodel.makeWikidataItemIdValue("Q1377"); private ItemIdValue qid2 = Datamodel.makeWikidataItemIdValue("Q865528"); diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValueTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValueTest.java index 7a649a964..d8cfddad6 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValueTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValueTest.java @@ -101,7 +101,7 @@ public class ReconEntityIdValueTest { // just checking this is symmetrical assertEquals(existingItem, Datamodel.makeWikidataItemIdValue("Q42")); - // new cell equality relies on the judgmentHistoryEntry parameter + // new item equality relies on the cell's recon id assertEquals(newItem, sameNewItem); assertNotEquals(newItem, differentNewItem); // and on datatype @@ -116,7 +116,7 @@ public class ReconEntityIdValueTest { @Test public void testGetRecon() { - assertEquals(newItem.getReconInternalId(), newItem.getRecon().judgmentHistoryEntry); + assertEquals(newItem.getReconInternalId(), newItem.getRecon().id); } @Test diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/testing/TestingData.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/testing/TestingData.java index 19ecd0fd2..3542824d2 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/testing/TestingData.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/testing/TestingData.java @@ -67,9 +67,17 @@ public class TestingData { public static ItemIdValue existingId = Datamodel.makeWikidataItemIdValue("Q43"); protected static PropertyIdValue pid = Datamodel.makeWikidataPropertyIdValue("P38"); + + public static class ReconStub extends Recon { + public ReconStub(long id, long judgmentHistoryEntry) { + super(id, judgmentHistoryEntry); + } + } - public static Recon makeNewItemRecon(long judgementId) { - Recon recon = Recon.makeWikidataRecon(judgementId); + public static Recon makeNewItemRecon(long id) { + Recon recon = new ReconStub(id, 382398L); // we keep the same judgment id because it is ignored + recon.identifierSpace = "http://www.wikidata.org/entity/"; + recon.schemaSpace = "http://www.wikidata.org/prop/direct/"; recon.judgment = Recon.Judgment.New; return recon; } diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/updates/scheduler/UpdateSchedulerTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/updates/scheduler/UpdateSchedulerTest.java index 9a6f29bc6..4f1828981 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/updates/scheduler/UpdateSchedulerTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/updates/scheduler/UpdateSchedulerTest.java @@ -42,8 +42,8 @@ public abstract class UpdateSchedulerTest { protected ItemIdValue existingIdA = Datamodel.makeWikidataItemIdValue("Q43"); protected ItemIdValue existingIdB = Datamodel.makeWikidataItemIdValue("Q538"); - protected ItemIdValue newIdA = TestingData.makeNewItemIdValue(1234L, "new item A"); - protected ItemIdValue newIdB = TestingData.makeNewItemIdValue(5678L, "new item B"); + protected ItemIdValue newIdA = TestingData.newIdA; + protected ItemIdValue newIdB = TestingData.newIdB; protected Statement sAtoB = TestingData.generateStatement(existingIdA, existingIdB); protected Statement sBtoA = TestingData.generateStatement(existingIdB, existingIdA);