From fa9d670d306e764ab4fd59a5621e100e394e26d2 Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Thu, 1 Apr 2021 13:58:28 +0200 Subject: [PATCH] Remove EditGroups URL template from Wikibase schema (#3779) * Remove EditGroups URL template from Wikibase schema Instead it is fetched from the manifest and stored in operation metadata. Closes #3656. * Fix compatibility with older JDKs * Remove unnecessary fallback in ManifestV1 constructor --- .../scripts/dialogs/import-schema-dialog.js | 1 - .../scripts/dialogs/perform-edits-dialog.js | 6 +++- .../module/scripts/schema-alignment.js | 3 +- .../commands/PerformWikibaseEditsCommand.java | 3 +- .../wikidata/manifests/Manifest.java | 1 + .../wikidata/manifests/ManifestV1.java | 11 ++++++ .../PerformWikibaseEditsOperation.java | 34 ++++++++++++++++--- .../wikidata/schema/WikibaseSchema.java | 16 ++------- .../tests/data/operations/perform-edits.json | 1 + .../wikidata/manifests/ManifestV1Test.java | 1 + .../PerformWikibaseEditsOperationTest.java | 2 +- .../wikidata/schema/WikibaseSchemaTest.java | 6 ++-- 12 files changed, 59 insertions(+), 26 deletions(-) diff --git a/extensions/wikidata/module/scripts/dialogs/import-schema-dialog.js b/extensions/wikidata/module/scripts/dialogs/import-schema-dialog.js index 6747e9de3..1160e7a3f 100644 --- a/extensions/wikidata/module/scripts/dialogs/import-schema-dialog.js +++ b/extensions/wikidata/module/scripts/dialogs/import-schema-dialog.js @@ -43,7 +43,6 @@ ImportSchemaDialog.launch = function() { if (!schema.siteIri || !schema.mediaWikiApiEndpoint) { schema.siteIri = WikidataManifestV1_0.wikibase.site_iri; schema.mediaWikiApiEndpoint = WikidataManifestV1_0.mediawiki.api; - schema.editGroupsURLSchema = WikidataManifestV1_0.editgroups.url_schema; } } catch(e) { elmts.invalidSchema.text($.i18n('import-wikibase-schema/invalid-schema')); diff --git a/extensions/wikidata/module/scripts/dialogs/perform-edits-dialog.js b/extensions/wikidata/module/scripts/dialogs/perform-edits-dialog.js index a34bd6f25..bd0d27c75 100644 --- a/extensions/wikidata/module/scripts/dialogs/perform-edits-dialog.js +++ b/extensions/wikidata/module/scripts/dialogs/perform-edits-dialog.js @@ -53,7 +53,11 @@ PerformEditsDialog.launch = function(logged_in_username, max_severity) { "wikidata", "perform-wikibase-edits", {}, - { summary: elmts.editSummary.val(), maxlag: elmts.maxlag.val() }, + { + summary: elmts.editSummary.val(), + maxlag: elmts.maxlag.val(), + editGroupsUrlSchema: WikibaseManager.getSelectedWikibaseEditGroupsURLSchema() + }, { includeEngine: true, cellsChanged: true, columnStatsChanged: true }, { onDone: function() { dismiss(); } } ); diff --git a/extensions/wikidata/module/scripts/schema-alignment.js b/extensions/wikidata/module/scripts/schema-alignment.js index d4726c77c..0c106c550 100644 --- a/extensions/wikidata/module/scripts/schema-alignment.js +++ b/extensions/wikidata/module/scripts/schema-alignment.js @@ -1279,8 +1279,7 @@ SchemaAlignment.getJSON = function() { return { itemDocuments: list, siteIri: WikibaseManager.getSelectedWikibaseSiteIri(), - mediaWikiApiEndpoint: WikibaseManager.getSelectedWikibaseApi(), - editGroupsURLSchema: WikibaseManager.getSelectedWikibaseEditGroupsURLSchema() + mediaWikiApiEndpoint: WikibaseManager.getSelectedWikibaseApi() }; } else { return null; diff --git a/extensions/wikidata/src/org/openrefine/wikidata/commands/PerformWikibaseEditsCommand.java b/extensions/wikidata/src/org/openrefine/wikidata/commands/PerformWikibaseEditsCommand.java index a0490d04e..bccee2c84 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/commands/PerformWikibaseEditsCommand.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/commands/PerformWikibaseEditsCommand.java @@ -40,7 +40,8 @@ public class PerformWikibaseEditsCommand extends EngineDependentCommand { String summary = request.getParameter("summary"); String maxlagStr = request.getParameter("maxlag"); int maxlag = maxlagStr == null ? 5 : Integer.parseInt(maxlagStr); - return new PerformWikibaseEditsOperation(engineConfig, summary, maxlag); + String editGroupsUrlSchema = request.getParameter("editGroupsUrlSchema"); + return new PerformWikibaseEditsOperation(engineConfig, summary, maxlag, editGroupsUrlSchema); } } diff --git a/extensions/wikidata/src/org/openrefine/wikidata/manifests/Manifest.java b/extensions/wikidata/src/org/openrefine/wikidata/manifests/Manifest.java index 215041192..821e78d40 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/manifests/Manifest.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/manifests/Manifest.java @@ -20,4 +20,5 @@ public interface Manifest { String getConstraintsRelatedId(String name); + String getEditGroupsUrlSchema(); } diff --git a/extensions/wikidata/src/org/openrefine/wikidata/manifests/ManifestV1.java b/extensions/wikidata/src/org/openrefine/wikidata/manifests/ManifestV1.java index f14e9a82e..e3f1171b1 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/manifests/ManifestV1.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/manifests/ManifestV1.java @@ -6,6 +6,8 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import org.wikidata.wdtk.wikibaseapi.ApiConnection; + public class ManifestV1 implements Manifest { private String version; @@ -16,6 +18,7 @@ public class ManifestV1 implements Manifest { private String subclassOfPid; private String mediaWikiApiEndpoint; private String reconServiceEndpoint; + private String editGroupsUrlSchema; private Map constraintsRelatedIdMap = new HashMap<>(); @@ -44,6 +47,9 @@ public class ManifestV1 implements Manifest { JsonNode reconciliation = manifest.path("reconciliation"); reconServiceEndpoint = reconciliation.path("endpoint").textValue(); + + JsonNode editGroups = manifest.path("editgroups"); + editGroupsUrlSchema = editGroups.path("url_schema").textValue(); } @Override @@ -91,4 +97,9 @@ public class ManifestV1 implements Manifest { return constraintsRelatedIdMap.get(name); } + @Override + public String getEditGroupsUrlSchema() { + return editGroupsUrlSchema; + } + } diff --git a/extensions/wikidata/src/org/openrefine/wikidata/operations/PerformWikibaseEditsOperation.java b/extensions/wikidata/src/org/openrefine/wikidata/operations/PerformWikibaseEditsOperation.java index f8c17b85e..e8075a838 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/operations/PerformWikibaseEditsOperation.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/operations/PerformWikibaseEditsOperation.java @@ -64,6 +64,10 @@ import com.google.refine.util.Pool; public class PerformWikibaseEditsOperation extends EngineDependentOperation { static final Logger logger = LoggerFactory.getLogger(PerformWikibaseEditsOperation.class); + + // only used for backwards compatibility, these things are configurable through + // the manifest now. + static final private String WIKIDATA_EDITGROUPS_URL_SCHEMA = "([[:toollabs:editgroups/b/OR/${batch_id}|details]])"; @JsonProperty("summary") private String summary; @@ -71,6 +75,9 @@ public class PerformWikibaseEditsOperation extends EngineDependentOperation { @JsonProperty("maxlag") private int maxlag; + @JsonProperty("editGroupsUrlSchema") + private String editGroupsUrlSchema; + @JsonCreator public PerformWikibaseEditsOperation( @JsonProperty("engineConfig") @@ -78,7 +85,9 @@ public class PerformWikibaseEditsOperation extends EngineDependentOperation { @JsonProperty("summary") String summary, @JsonProperty("maxlag") - Integer maxlag) { + Integer maxlag, + @JsonProperty("editGroupsUrlSchema") + String editGroupsUrlSchema) { super(engineConfig); Validate.notNull(summary, "An edit summary must be provided."); Validate.notEmpty(summary, "An edit summary must be provided."); @@ -89,6 +98,8 @@ public class PerformWikibaseEditsOperation extends EngineDependentOperation { maxlag = 5; } this.maxlag = maxlag; + // a fallback to Wikidata for backwards compatibility is done later on + this.editGroupsUrlSchema = editGroupsUrlSchema; } @Override @@ -99,7 +110,12 @@ public class PerformWikibaseEditsOperation extends EngineDependentOperation { @Override public Process createProcess(Project project, Properties options) throws Exception { - return new PerformEditsProcess(project, createEngine(project), getBriefDescription(project), summary); + return new PerformEditsProcess( + project, + createEngine(project), + getBriefDescription(project), + editGroupsUrlSchema, + summary); } static public class PerformWikibaseEditsChange implements Change { @@ -158,11 +174,12 @@ public class PerformWikibaseEditsOperation extends EngineDependentOperation { protected Project _project; protected Engine _engine; protected WikibaseSchema _schema; + protected String _editGroupsUrlSchema; protected String _summary; protected List _tags; protected final long _historyEntryID; - protected PerformEditsProcess(Project project, Engine engine, String description, String summary) { + protected PerformEditsProcess(Project project, Engine engine, String description, String editGroupsUrlSchema, String summary) { super(description); this._project = project; this._engine = engine; @@ -176,6 +193,13 @@ public class PerformWikibaseEditsOperation extends EngineDependentOperation { } this._tags = Arrays.asList(tag); this._historyEntryID = HistoryEntry.allocateID(); + if (editGroupsUrlSchema == null && + ApiConnection.URL_WIKIDATA_API.equals(_schema.getMediaWikiApiEndpoint())) { + // For backward compatibility, if no editGroups schema is provided + // and we edit Wikidata, then add Wikidata's editGroups schema + editGroupsUrlSchema = WIKIDATA_EDITGROUPS_URL_SCHEMA; + } + this._editGroupsUrlSchema = editGroupsUrlSchema; } @Override @@ -193,7 +217,7 @@ public class PerformWikibaseEditsOperation extends EngineDependentOperation { WikibaseDataEditor wbde = new WikibaseDataEditor(connection, _schema.getSiteIri()); String summary; - if (StringUtils.isBlank(_schema.getEditGroupsURLSchema())) { + if (StringUtils.isBlank(_editGroupsUrlSchema)) { summary = _summary; } else { // Generate batch id @@ -203,7 +227,7 @@ public class PerformWikibaseEditsOperation extends EngineDependentOperation { // from the user-supplied ones, we replace these separators by similar unicode characters to // make sure they can be told apart. String summaryWithoutCommas = _summary.replaceAll(", ","ꓹ ").replaceAll(": ","։ "); - summary = summaryWithoutCommas + " " + _schema.getEditGroupsURLSchema().replace("${batch_id}", batchId); + summary = summaryWithoutCommas + " " + _editGroupsUrlSchema.replace("${batch_id}", batchId); } // Evaluate the schema diff --git a/extensions/wikidata/src/org/openrefine/wikidata/schema/WikibaseSchema.java b/extensions/wikidata/src/org/openrefine/wikidata/schema/WikibaseSchema.java index 82f0bc734..68b92f657 100644 --- a/extensions/wikidata/src/org/openrefine/wikidata/schema/WikibaseSchema.java +++ b/extensions/wikidata/src/org/openrefine/wikidata/schema/WikibaseSchema.java @@ -28,12 +28,12 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import com.fasterxml.jackson.annotation.JsonIgnore; import org.openrefine.wikidata.qa.QAWarningStore; import org.openrefine.wikidata.schema.exceptions.SkipSchemaExpressionException; import org.openrefine.wikidata.updates.ItemUpdate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.wikidata.wdtk.wikibaseapi.ApiConnection; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; @@ -67,9 +67,6 @@ public class WikibaseSchema implements OverlayModel { @JsonProperty("mediaWikiApiEndpoint") protected String mediaWikiApiEndpoint; - @JsonIgnore - protected String editGroupsURLSchema; - /** * Constructor. */ @@ -83,12 +80,10 @@ public class WikibaseSchema implements OverlayModel { @JsonCreator public WikibaseSchema(@JsonProperty("itemDocuments") List exprs, @JsonProperty("siteIri") String siteIri, - @JsonProperty("mediaWikiApiEndpoint") String mediaWikiApiEndpoint, - @JsonProperty("editGroupsURLSchema") String editGroupsURLSchema) { + @JsonProperty("mediaWikiApiEndpoint") String mediaWikiApiEndpoint) { this.itemDocumentExprs = exprs; this.siteIri = siteIri; - this.mediaWikiApiEndpoint = mediaWikiApiEndpoint; - this.editGroupsURLSchema = editGroupsURLSchema; + this.mediaWikiApiEndpoint = mediaWikiApiEndpoint != null ? mediaWikiApiEndpoint : ApiConnection.URL_WIKIDATA_API; } /** @@ -112,11 +107,6 @@ public class WikibaseSchema implements OverlayModel { return mediaWikiApiEndpoint; } - @JsonIgnore - public String getEditGroupsURLSchema() { - return editGroupsURLSchema; - } - /** * Evaluates all item documents in a particular expression context. This * specifies, among others, a row where the values of the variables will be diff --git a/extensions/wikidata/tests/data/operations/perform-edits.json b/extensions/wikidata/tests/data/operations/perform-edits.json index 0d6ded5c3..d223deb5a 100644 --- a/extensions/wikidata/tests/data/operations/perform-edits.json +++ b/extensions/wikidata/tests/data/operations/perform-edits.json @@ -3,6 +3,7 @@ "description": "Perform Wikibase edits", "summary": "test null edit", "maxlag": 5, + "editGroupsUrlSchema": "([[:toollabs:editgroups/b/OR/${batch_id}|details]])", "engineConfig": { "mode": "row-based", "facets": [] diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/manifests/ManifestV1Test.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/manifests/ManifestV1Test.java index d6858c2b2..b34260b50 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/manifests/ManifestV1Test.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/manifests/ManifestV1Test.java @@ -23,6 +23,7 @@ public class ManifestV1Test { assertEquals("https://wdreconcile.toolforge.org/${lang}/api", manifest.getReconServiceEndpoint()); assertEquals("P2302", manifest.getConstraintsRelatedId("property_constraint_pid")); assertEquals("Q19474404", manifest.getConstraintsRelatedId("single_value_constraint_qid")); + assertEquals("([[:toollabs:editgroups/b/OR/${batch_id}|details]])", manifest.getEditGroupsUrlSchema()); } @Test diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/operations/PerformWikibaseEditsOperationTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/operations/PerformWikibaseEditsOperationTest.java index 2fca765bb..391ab50b6 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/operations/PerformWikibaseEditsOperationTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/operations/PerformWikibaseEditsOperationTest.java @@ -58,7 +58,7 @@ public class PerformWikibaseEditsOperationTest extends OperationTest { @Test(expectedExceptions=IllegalArgumentException.class) public void testConstructor() { - new PerformWikibaseEditsOperation(EngineConfig.reconstruct("{}"), "", 5); + new PerformWikibaseEditsOperation(EngineConfig.reconstruct("{}"), "", 5, ""); } @Test diff --git a/extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WikibaseSchemaTest.java b/extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WikibaseSchemaTest.java index ad3fde52a..b6413fa69 100644 --- a/extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WikibaseSchemaTest.java +++ b/extensions/wikidata/tests/src/org/openrefine/wikidata/schema/WikibaseSchemaTest.java @@ -23,7 +23,6 @@ ******************************************************************************/ package org.openrefine.wikidata.schema; -import org.testng.Assert; import static org.testng.Assert.assertEquals; import java.io.IOException; @@ -48,6 +47,7 @@ import org.wikidata.wdtk.datamodel.interfaces.Statement; import org.wikidata.wdtk.datamodel.interfaces.StatementRank; import org.wikidata.wdtk.datamodel.interfaces.StringValue; import org.wikidata.wdtk.datamodel.interfaces.TimeValue; +import org.wikidata.wdtk.wikibaseapi.ApiConnection; import com.google.refine.browsing.Engine; import com.google.refine.browsing.EngineConfig; @@ -106,7 +106,9 @@ public class WikibaseSchemaTest extends WikidataRefineTest { // this json file was generated by an earlier version of the software // it contains extra "type" fields that are now ignored. String serialized = TestingData.jsonFromFile("schema/roarmap.json"); - WikibaseSchema.reconstruct(serialized); + WikibaseSchema schema = WikibaseSchema.reconstruct(serialized); + // Check that we fall back on Wikidata if no API endpoint was supplied + assertEquals(schema.getMediaWikiApiEndpoint(), ApiConnection.URL_WIKIDATA_API); } @Test