From 03c767485831763324cbedbc55a88adae1b646ce Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Sat, 3 Mar 2018 23:06:21 +0000 Subject: [PATCH] Fix siteIRI of new item ids (switch to SITE_LOCAL) --- .../wikidata/editing/EditBatchProcessor.java | 5 +- .../wikidata/editing/ReconEntityRewriter.java | 2 +- .../entityvalues/ReconEntityIdValue.java | 49 +++++-------------- .../wikidata/updates/ItemUpdate.java | 2 +- .../editing/EditBatchProcessorTest.java | 4 +- .../entityvalues/ReconEntityIdValueTest.java | 9 ++-- 6 files changed, 23 insertions(+), 48 deletions(-) diff --git a/extensions/wikidata/src/org/openrefine/wikidata/editing/EditBatchProcessor.java b/extensions/wikidata/src/org/openrefine/wikidata/editing/EditBatchProcessor.java index 97f687263..ed036ff4b 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/editing/EditBatchProcessor.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/editing/EditBatchProcessor.java @@ -15,7 +15,6 @@ import org.slf4j.LoggerFactory; import org.wikidata.wdtk.datamodel.helpers.Datamodel; import org.wikidata.wdtk.datamodel.interfaces.EntityDocument; import org.wikidata.wdtk.datamodel.interfaces.ItemDocument; -import org.wikidata.wdtk.datamodel.interfaces.ItemIdValue; import org.wikidata.wdtk.datamodel.interfaces.MonolingualTextValue; import org.wikidata.wdtk.wikibaseapi.WikibaseDataEditor; import org.wikidata.wdtk.wikibaseapi.WikibaseDataFetcher; @@ -112,9 +111,9 @@ public class EditBatchProcessor { // New item if (update.isNew()) { ReconEntityIdValue newCell = (ReconEntityIdValue)update.getItemId(); - update.normalizeLabelsAndAliases(); + update = update.normalizeLabelsAndAliases(); - ItemDocument itemDocument = Datamodel.makeItemDocument(ItemIdValue.NULL, + ItemDocument itemDocument = Datamodel.makeItemDocument(update.getItemId(), update.getLabels().stream().collect(Collectors.toList()), update.getDescriptions().stream().collect(Collectors.toList()), update.getAliases().stream().collect(Collectors.toList()), diff --git a/extensions/wikidata/src/org/openrefine/wikidata/editing/ReconEntityRewriter.java b/extensions/wikidata/src/org/openrefine/wikidata/editing/ReconEntityRewriter.java index 493664914..0b28edc5b 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/editing/ReconEntityRewriter.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/editing/ReconEntityRewriter.java @@ -65,7 +65,7 @@ public class ReconEntityRewriter extends DatamodelConverter { "Trying to rewrite an update where a new item was not created yet."); } return Datamodel.makeItemIdValue(newId, - recon.getSiteIri()); + recon.getRecon().identifierSpace); } } return super.copy(value); 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 4975aa98a..7f533e892 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValue.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/schema/entityvalues/ReconEntityIdValue.java @@ -4,6 +4,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.wikidata.wdtk.datamodel.helpers.Equality; import org.wikidata.wdtk.datamodel.helpers.Hash; import org.wikidata.wdtk.datamodel.interfaces.EntityIdValue; import org.wikidata.wdtk.datamodel.interfaces.ValueVisitor; @@ -87,7 +88,7 @@ public abstract class ReconEntityIdValue implements PrefetchedEntityIdValue { * @return * the full reconciliation metadata of the corresponding cell */ - protected Recon getRecon() { + public Recon getRecon() { return _recon; } @@ -99,16 +100,20 @@ public abstract class ReconEntityIdValue implements PrefetchedEntityIdValue { if (isMatched()) { return _recon.match.id; } else if (ET_ITEM.equals(getEntityType())) { - return "Q0"; + return "Q"+getReconInternalId(); } else if (ET_PROPERTY.equals(getEntityType())) { - return "P0"; + return "P"+getReconInternalId(); } return null; } @Override public String getSiteIri() { - return _recon.identifierSpace; + if (isMatched()) { + return _recon.identifierSpace; + } else { + return EntityIdValue.SITE_LOCAL; + } } @Override @@ -121,45 +126,15 @@ public abstract class ReconEntityIdValue implements PrefetchedEntityIdValue { return valueVisitor.visit(this); } - - /** - * Equality check is important when we gather - * all ItemUpdates related to an ItemId. - * - * The label is ignored in the equality check. - */ @Override public boolean equals(Object other) { - if (other == null || - !EntityIdValue.class.isInstance(other)) { - return false; - } - - if (ReconEntityIdValue.class.isInstance(other)) { - final ReconEntityIdValue reconOther = (ReconEntityIdValue)other; - - if (isNew() != reconOther.isNew()) { - return false; - } else if (isNew()) { - // This ensures compliance with OR's notion of new items - // (it is possible that two cells are reconciled to the same - // new item, in which case they share the same internal recon id). - return (getReconInternalId() == reconOther.getReconInternalId() && - getEntityType().equals(reconOther.getEntityType())); - } - } - - final EntityIdValue otherNew = (EntityIdValue)other; - return getIri().equals(otherNew.getIri()); + return Equality.equalsEntityIdValue(this, other); + } @Override public int hashCode() { - if (isMatched()) { - return Hash.hashCode(this); - } else { - return (int) getReconInternalId(); - } + return Hash.hashCode(this); } @Override diff --git a/extensions/wikidata/src/org/openrefine/wikidata/updates/ItemUpdate.java b/extensions/wikidata/src/org/openrefine/wikidata/updates/ItemUpdate.java index 670cd8e36..55a582ff0 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/updates/ItemUpdate.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/updates/ItemUpdate.java @@ -238,7 +238,7 @@ public class ItemUpdate { * Is this update about a new item? */ public boolean isNew() { - return "Q0".equals(getItemId().getId()); + return EntityIdValue.SITE_LOCAL.equals(getItemId().getSiteIri()); } /** diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/editing/EditBatchProcessorTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/editing/EditBatchProcessorTest.java index a94ef5b64..7278268eb 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/editing/EditBatchProcessorTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/editing/EditBatchProcessorTest.java @@ -56,7 +56,7 @@ public class EditBatchProcessorTest extends RefineTest { .build()); MonolingualTextValue label = Datamodel.makeMonolingualTextValue("better label", "en"); batch.add(new ItemUpdateBuilder(TestingData.newIdA) - .addLabel(label) + .addAlias(label) .build()); // Plan expected edits @@ -67,7 +67,7 @@ public class EditBatchProcessorTest extends RefineTest { when(fetcher.getEntityDocuments(Collections.singletonList(TestingData.existingId.getId()))) .thenReturn(Collections.singletonMap(TestingData.existingId.getId(), existingItem)); - ItemDocument expectedNewItem = ItemDocumentBuilder.forItemId(ItemIdValue.NULL) + ItemDocument expectedNewItem = ItemDocumentBuilder.forItemId(TestingData.newIdA) .withLabel(label).build(); ItemDocument createdNewItem = ItemDocumentBuilder.forItemId(Datamodel.makeWikidataItemIdValue("Q1234")) .withLabel(label).withRevisionId(37828L).build(); 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 9ff52c101..1cda7cf4c 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 @@ -10,6 +10,7 @@ import java.util.Collections; import org.openrefine.wikidata.testing.TestingData; import org.testng.annotations.Test; import org.wikidata.wdtk.datamodel.helpers.Datamodel; +import org.wikidata.wdtk.datamodel.interfaces.EntityIdValue; import com.google.refine.model.Recon; @@ -47,21 +48,21 @@ public class ReconEntityIdValueTest { @Test public void testGetId() { assertEquals("Q42", existingItem.getId()); - assertEquals("Q0", newItem.getId()); + assertEquals("Q1234", newItem.getId()); assertEquals("P53", existingProp.getId()); - assertEquals("P0", newProp.getId()); + assertEquals("P1234", newProp.getId()); } @Test public void testGetIri() { assertEquals("http://www.wikidata.org/entity/Q42", existingItem.getIri()); - assertEquals("http://www.wikidata.org/entity/Q0", newItem.getIri()); + assertEquals(EntityIdValue.SITE_LOCAL+"Q1234", newItem.getIri()); } @Test public void testGetSiteIri() { assertEquals("http://www.wikidata.org/entity/", existingItem.getSiteIri()); - assertEquals("http://www.wikidata.org/entity/", newItem.getSiteIri()); + assertEquals(EntityIdValue.SITE_LOCAL, newItem.getSiteIri()); } @Test