diff --git a/extensions/wikidata/src/org/openrefine/wikidata/qa/ConstraintFetcher.java b/extensions/wikidata/src/org/openrefine/wikidata/qa/ConstraintFetcher.java index 9933bb02d..44457a86d 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/qa/ConstraintFetcher.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/qa/ConstraintFetcher.java @@ -55,19 +55,19 @@ public interface ConstraintFetcher { PropertyIdValue getInversePid(PropertyIdValue pid); /** - * Is this property for values only? + * Can this property be used as values? */ - boolean isForValuesOnly(PropertyIdValue pid); + boolean allowedAsValue(PropertyIdValue pid); /** - * Is this property for qualifiers only? + * Can this property be used as qualifiers? */ - boolean isForQualifiersOnly(PropertyIdValue pid); + boolean allowedAsQualifier(PropertyIdValue pid); /** - * Is this property for references only? + * Can this property be used in a reference? */ - boolean isForReferencesOnly(PropertyIdValue pid); + boolean allowedAsReference(PropertyIdValue pid); /** * Get the list of allowed qualifiers (as property ids) for this property (null diff --git a/extensions/wikidata/src/org/openrefine/wikidata/qa/WikidataConstraintFetcher.java b/extensions/wikidata/src/org/openrefine/wikidata/qa/WikidataConstraintFetcher.java index 8b301a530..be06540eb 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/qa/WikidataConstraintFetcher.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/qa/WikidataConstraintFetcher.java @@ -30,7 +30,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.openrefine.wikidata.utils.EntityCache; +import org.wikidata.wdtk.datamodel.helpers.Datamodel; import org.wikidata.wdtk.datamodel.interfaces.EntityIdValue; +import org.wikidata.wdtk.datamodel.interfaces.ItemIdValue; import org.wikidata.wdtk.datamodel.interfaces.PropertyDocument; import org.wikidata.wdtk.datamodel.interfaces.PropertyIdValue; import org.wikidata.wdtk.datamodel.interfaces.Snak; @@ -57,7 +59,11 @@ public class WikidataConstraintFetcher implements ConstraintFetcher { public static String INVERSE_CONSTRAINT_QID = "Q21510855"; public static String INVERSE_PROPERTY_PID = "P2306"; - public static String USED_ONLY_AS_VALUES_CONSTRAINT_QID = "Q21528958"; + public static String SCOPE_CONSTRAINT_QID = "Q53869507"; + public static String SCOPE_CONSTRAINT_PID = "P5314"; + public static String SCOPE_CONSTRAINT_VALUE_QID = "Q54828448"; + public static String SCOPE_CONSTRAINT_QUALIFIER_QID = "Q54828449"; + public static String SCOPE_CONSTRAINT_REFERENCE_QID = "Q54828450"; public static String USED_ONLY_AS_QUALIFIER_CONSTRAINT_QID = "Q21510863"; @@ -102,18 +108,36 @@ public class WikidataConstraintFetcher implements ConstraintFetcher { } @Override - public boolean isForValuesOnly(PropertyIdValue pid) { - return getSingleConstraint(pid, USED_ONLY_AS_VALUES_CONSTRAINT_QID) != null; + public boolean allowedAsValue(PropertyIdValue pid) { + List specs = getSingleConstraint(pid, SCOPE_CONSTRAINT_QID); + + if (specs != null) { + ItemIdValue target = Datamodel.makeWikidataItemIdValue(SCOPE_CONSTRAINT_VALUE_QID); + return findValues(specs, SCOPE_CONSTRAINT_PID).contains(target); + } + return true; } @Override - public boolean isForQualifiersOnly(PropertyIdValue pid) { - return getSingleConstraint(pid, USED_ONLY_AS_QUALIFIER_CONSTRAINT_QID) != null; + public boolean allowedAsQualifier(PropertyIdValue pid) { + List specs = getSingleConstraint(pid, SCOPE_CONSTRAINT_QID); + + if (specs != null) { + ItemIdValue target = Datamodel.makeWikidataItemIdValue(SCOPE_CONSTRAINT_QUALIFIER_QID); + return findValues(specs, SCOPE_CONSTRAINT_PID).contains(target); + } + return true; } @Override - public boolean isForReferencesOnly(PropertyIdValue pid) { - return getSingleConstraint(pid, USED_ONLY_AS_REFERENCE_CONSTRAINT_QID) != null; + public boolean allowedAsReference(PropertyIdValue pid) { + List specs = getSingleConstraint(pid, SCOPE_CONSTRAINT_QID); + + if (specs != null) { + ItemIdValue target = Datamodel.makeWikidataItemIdValue(SCOPE_CONSTRAINT_REFERENCE_QID); + return findValues(specs, SCOPE_CONSTRAINT_PID).contains(target); + } + return true; } @Override diff --git a/extensions/wikidata/src/org/openrefine/wikidata/qa/scrutinizers/RestrictedPositionScrutinizer.java b/extensions/wikidata/src/org/openrefine/wikidata/qa/scrutinizers/RestrictedPositionScrutinizer.java index fa95b12c0..fc09e134a 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/qa/scrutinizers/RestrictedPositionScrutinizer.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/qa/scrutinizers/RestrictedPositionScrutinizer.java @@ -23,11 +23,7 @@ ******************************************************************************/ package org.openrefine.wikidata.qa.scrutinizers; -import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; -import java.util.Map; -import java.util.Set; import org.openrefine.wikidata.qa.QAWarning; import org.wikidata.wdtk.datamodel.interfaces.EntityIdValue; @@ -42,40 +38,6 @@ public class RestrictedPositionScrutinizer extends StatementScrutinizer { MAINSNAK, QUALIFIER, REFERENCE } - private Map _restrictedPids; - private Set _unrestrictedPids; - - public RestrictedPositionScrutinizer() { - _restrictedPids = new HashMap<>(); - _unrestrictedPids = new HashSet<>(); - } - - SnakPosition positionRestriction(PropertyIdValue pid) { - if (_unrestrictedPids.contains(pid)) { - return null; - } - SnakPosition restriction = _restrictedPids.get(pid); - if (restriction != null) { - return restriction; - } else { - if (_fetcher.isForValuesOnly(pid)) { - restriction = SnakPosition.MAINSNAK; - } else if (_fetcher.isForQualifiersOnly(pid)) { - restriction = SnakPosition.QUALIFIER; - } else if (_fetcher.isForReferencesOnly(pid)) { - restriction = SnakPosition.REFERENCE; - } - - // Cache these results: - if (restriction != null) { - _restrictedPids.put(pid, restriction); - } else { - _unrestrictedPids.add(pid); - } - return restriction; - } - } - @Override public void scrutinize(Statement statement, EntityIdValue entityId, boolean added) { // Skip the main snak @@ -99,16 +61,25 @@ public class RestrictedPositionScrutinizer extends StatementScrutinizer { } public void scrutinize(Snak snak, EntityIdValue entityId, SnakPosition position, boolean added) { - SnakPosition restriction = positionRestriction(snak.getPropertyId()); - if (restriction != null && position != restriction) { + if (!positionAllowed(snak.getPropertyId(), position)) { String positionStr = position.toString().toLowerCase(); - String restrictionStr = restriction.toString().toLowerCase(); - QAWarning issue = new QAWarning("property-restricted-to-" + restrictionStr + "-found-in-" + positionStr, + QAWarning issue = new QAWarning("property-found-in-" + positionStr, snak.getPropertyId().getId(), QAWarning.Severity.IMPORTANT, 1); issue.setProperty("property_entity", snak.getPropertyId()); addIssue(issue); } } + + public boolean positionAllowed(PropertyIdValue pid, SnakPosition position) { + if(position.equals(SnakPosition.MAINSNAK)) { + return _fetcher.allowedAsValue(pid); + } else if(position.equals(SnakPosition.QUALIFIER)) { + return _fetcher.allowedAsQualifier(pid); + } else if(position.equals(SnakPosition.REFERENCE)) { + return _fetcher.allowedAsReference(pid); + } + return true; + } } diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/qa/MockConstraintFetcher.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/qa/MockConstraintFetcher.java index b13c7ccff..af2a3a3be 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/qa/MockConstraintFetcher.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/qa/MockConstraintFetcher.java @@ -60,18 +60,18 @@ public class MockConstraintFetcher implements ConstraintFetcher { } @Override - public boolean isForValuesOnly(PropertyIdValue pid) { - return mainSnakPid.equals(pid); + public boolean allowedAsValue(PropertyIdValue pid) { + return (!qualifierPid.equals(pid) && !referencePid.equals(pid)); } @Override - public boolean isForQualifiersOnly(PropertyIdValue pid) { - return qualifierPid.equals(pid); + public boolean allowedAsQualifier(PropertyIdValue pid) { + return (!mainSnakPid.equals(pid) && !referencePid.equals(pid)); } @Override - public boolean isForReferencesOnly(PropertyIdValue pid) { - return referencePid.equals(pid); + public boolean allowedAsReference(PropertyIdValue pid) { + return (!mainSnakPid.equals(pid) && !qualifierPid.equals(pid)); } @Override diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/qa/WikidataConstraintFetcherTests.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/qa/WikidataConstraintFetcherTests.java index 7f4df52a7..ed90e9833 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/qa/WikidataConstraintFetcherTests.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/qa/WikidataConstraintFetcherTests.java @@ -39,10 +39,7 @@ public class WikidataConstraintFetcherTests { private PropertyIdValue endTime; private PropertyIdValue instanceOf; private PropertyIdValue gridId; - private PropertyIdValue hasPart; private PropertyIdValue partOf; - private PropertyIdValue referenceURL; - private PropertyIdValue reasonForDeprecation; private PropertyIdValue mother; private PropertyIdValue child; @@ -53,10 +50,7 @@ public class WikidataConstraintFetcherTests { endTime = Datamodel.makeWikidataPropertyIdValue("P582"); instanceOf = Datamodel.makeWikidataPropertyIdValue("P31"); gridId = Datamodel.makeWikidataPropertyIdValue("P2427"); - hasPart = Datamodel.makeWikidataPropertyIdValue("P527"); partOf = Datamodel.makeWikidataPropertyIdValue("P361"); - referenceURL = Datamodel.makeWikidataPropertyIdValue("P854"); - reasonForDeprecation = Datamodel.makeWikidataPropertyIdValue("P2241"); mother = Datamodel.makeWikidataPropertyIdValue("P25"); child = Datamodel.makeWikidataPropertyIdValue("P40"); } @@ -77,24 +71,6 @@ public class WikidataConstraintFetcherTests { Assert.assertEquals(fetcher.getInversePid(mother), child); } - @Test - public void testOnlyReferences() { - Assert.assertTrue(fetcher.isForReferencesOnly(referenceURL)); - Assert.assertFalse(fetcher.isForReferencesOnly(reasonForDeprecation)); - } - - @Test - public void testOnlyQualifiers() { - Assert.assertTrue(fetcher.isForQualifiersOnly(reasonForDeprecation)); - Assert.assertFalse(fetcher.isForQualifiersOnly(headOfGovernment)); - } - - @Test - public void testOnlyValues() { - Assert.assertTrue(fetcher.isForValuesOnly(headOfGovernment)); - Assert.assertFalse(fetcher.isForValuesOnly(referenceURL)); - } - @Test public void testAllowedQualifiers() { Assert.assertTrue(fetcher.allowedQualifiers(headOfGovernment).contains(startTime));