Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wikibase: Better handling of the 'badtags' error #6552

Merged
merged 2 commits into from May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion extensions/wikibase/module/scripts/wikibase-manager.js
Expand Up @@ -51,7 +51,7 @@ WikibaseManager.getSelectedWikibaseMaxlag = function() {

WikibaseManager.getSelectedWikibaseTagTemplate = function() {
let tag = WikibaseManager.getSelectedWikibase().wikibase.tag;
return tag === undefined ? 'openrefine-${version}' : tag;
return tag === undefined ? 'openrefine' : tag;
};

WikibaseManager.getSelectedWikibaseMaxEditsPerMinute = function() {
Expand Down
Expand Up @@ -27,6 +27,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -65,8 +66,9 @@ public class EditBatchProcessor {
private NewEntityLibrary library;
private List<EntityEdit> scheduled;
private String summary;
private List<String> tags;
private LinkedList<String> tagCandidates;

private String currentTag;
private List<EntityEdit> remainingUpdates;
private List<EntityEdit> currentBatch;
private int batchCursor;
Expand All @@ -90,16 +92,16 @@ public class EditBatchProcessor {
* the library to use to keep track of new entity creation
* @param summary
* the summary to append to all edits
* @param tags
* the list of tags to apply to all edits
* @param tagCandidates
* the list of tags to try to apply to edits. The first existing tag will be added to all edits (if any).
* @param batchSize
* the number of entities that should be retrieved in one go from the API
* @param maxEditsPerMinute
* the maximum number of edits per minute to do
*/
public EditBatchProcessor(WikibaseDataFetcher fetcher, WikibaseDataEditor editor, ApiConnection connection,
List<EntityEdit> entityDocuments,
NewEntityLibrary library, String summary, int maxLag, List<String> tags, int batchSize, int maxEditsPerMinute) {
NewEntityLibrary library, String summary, int maxLag, List<String> tagCandidates, int batchSize, int maxEditsPerMinute) {
this.fetcher = fetcher;
this.editor = editor;
this.connection = connection;
Expand All @@ -114,7 +116,7 @@ public EditBatchProcessor(WikibaseDataFetcher fetcher, WikibaseDataEditor editor

this.library = library;
this.summary = summary;
this.tags = tags;
this.tagCandidates = new LinkedList<>(tagCandidates);
this.batchSize = batchSize;

// Schedule the edit batch
Expand Down Expand Up @@ -158,6 +160,12 @@ public void performEdit()
return;
}

// Pick a tag to apply to the edits
if (currentTag == null && !tagCandidates.isEmpty()) {
currentTag = tagCandidates.remove();
}
List<String> tags = currentTag == null ? Collections.emptyList() : Collections.singletonList(currentTag);

try {
if (update.isNew()) {
// New entities
Expand Down Expand Up @@ -203,9 +211,16 @@ public void performEdit()
}
}
} catch (MediaWikiApiErrorException e) {
// TODO find a way to report these errors to the user in a nice way
logger.warn("MediaWiki error while editing [" + e.getErrorCode()
+ "]: " + e.getErrorMessage());
if ("badtags".equals(e.getErrorCode()) && currentTag != null) {
// if we tried editing with a tag that does not exist, clear the tag and try again
currentTag = null;
performEdit();
return;
} else {
// TODO find a way to report these errors to the user in a nice way
logger.warn("MediaWiki error while editing [" + e.getErrorCode()
+ "]: " + e.getErrorMessage());
}
} catch (IOException e) {
logger.warn("IO error while editing: " + e.getMessage());
}
Expand Down
Expand Up @@ -16,7 +16,7 @@ public interface Manifest {
public static final String PROPERTY_TYPE = "property";
public static final String MEDIAINFO_TYPE = "mediainfo";
public static final int DEFAULT_MAX_EDITS_PER_MINUTE = 60;
public static final String DEFAULT_TAG_TEMPLATE = "openrefine-${version}";
public static final String DEFAULT_TAG_TEMPLATE = "openrefine";

/**
* The version of the manifest object, which determines its JSON format.
Expand Down
Expand Up @@ -27,14 +27,15 @@
import java.io.IOException;
import java.io.LineNumberReader;
import java.io.Writer;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Properties;
import java.util.Random;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -128,6 +129,36 @@ public Process createProcess(Project project, Properties options)
summary);
}

/**
* @return the list of tags which we should attempt to add to our edits. The first existing tag in the list will be
* used. If none of the returned tags exist, editing will happen without any tag.
*/
@JsonIgnore
protected List<String> getTagCandidates(String refineVersion) {
List<String> results = new LinkedList<>();

if (tagTemplate.contains("${version}")) {
Pattern pattern = Pattern.compile("^(\\d+\\.\\d+).*$");
Matcher matcher = pattern.matcher(refineVersion);
if (matcher.matches()) {
results.add(tagTemplate.replace("${version}", matcher.group(1)));
}

// if the tag template includes the version, also add a version-independent tag as fallback.
// for instance, if the tag template is `openrefine-${version}`, also try adding the tag `openrefine`
// in case the version-specific tag isn't available.
// See https://github.com/OpenRefine/OpenRefine/issues/6551
if (tagTemplate.endsWith("-${version}")) {
results.add(tagTemplate.substring(0, tagTemplate.length() - "-${version}".length()));
}

} else {
results.add(tagTemplate);
}

return results;
}

static public class PerformWikibaseEditsChange implements Change {

private NewEntityLibrary newEntityLibrary;
Expand Down Expand Up @@ -186,7 +217,6 @@ public class PerformEditsProcess extends LongRunningProcess implements Runnable
protected WikibaseSchema _schema;
protected String _editGroupsUrlSchema;
protected String _summary;
protected List<String> _tags;
protected final long _historyEntryID;

protected PerformEditsProcess(Project project, Engine engine, String description, String editGroupsUrlSchema, String summary) {
Expand All @@ -195,15 +225,6 @@ protected PerformEditsProcess(Project project, Engine engine, String description
this._engine = engine;
this._schema = (WikibaseSchema) project.overlayModels.get("wikibaseSchema");
this._summary = summary;
String tag = tagTemplate;
if (tag.contains("${version}")) {
Pattern pattern = Pattern.compile("^(\\d+\\.\\d+).*$");
Matcher matcher = pattern.matcher(RefineServlet.VERSION);
if (matcher.matches()) {
tag = tag.replace("${version}", matcher.group(1));
}
}
this._tags = tag.isEmpty() ? Collections.emptyList() : Collections.singletonList(tag);
this._historyEntryID = HistoryEntry.allocateID();
if (editGroupsUrlSchema == null &&
ApiConnection.URL_WIKIDATA_API.equals(_schema.getMediaWikiApiEndpoint())) {
Expand Down Expand Up @@ -254,7 +275,7 @@ public void run() {
// Prepare the edits
NewEntityLibrary newEntityLibrary = new NewEntityLibrary();
EditBatchProcessor processor = new EditBatchProcessor(fetcher, editor, connection, entityDocuments, newEntityLibrary, summary,
maxlag, _tags, 50, maxEditsPerMinute);
maxlag, getTagCandidates(RefineServlet.VERSION), 50, maxEditsPerMinute);

// Perform edits
logger.info("Performing edits");
Expand Down
Expand Up @@ -26,6 +26,7 @@

import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -54,6 +55,7 @@
import org.wikidata.wdtk.datamodel.interfaces.StatementUpdate;
import org.wikidata.wdtk.datamodel.interfaces.TermUpdate;
import org.wikidata.wdtk.wikibaseapi.ApiConnection;
import org.wikidata.wdtk.wikibaseapi.EditingResult;
import org.wikidata.wdtk.wikibaseapi.WikibaseDataEditor;
import org.wikidata.wdtk.wikibaseapi.WikibaseDataFetcher;
import org.wikidata.wdtk.wikibaseapi.apierrors.MediaWikiApiErrorException;
Expand Down Expand Up @@ -126,6 +128,60 @@ public void testNewItem()
assertEquals(expectedLibrary, library);
}

@Test
public void testFallbackToSecondTag() throws Exception {
List<EntityEdit> batch = new ArrayList<>();
batch.add(new ItemEditBuilder(TestingData.existingId)
.addAlias(Datamodel.makeMonolingualTextValue("my new alias", "en"))
.build());
ItemDocument existingItem = ItemDocumentBuilder.forItemId(TestingData.existingId)
.withLabel(Datamodel.makeMonolingualTextValue("pomme", "fr"))
.withDescription(Datamodel.makeMonolingualTextValue("fruit délicieux", "fr")).build();
when(fetcher.getEntityDocuments(Collections.singletonList(TestingData.existingId.getId())))
.thenReturn(Collections.singletonMap(TestingData.existingId.getId(), existingItem));

when(editor.editEntityDocument(any(), anyBoolean(), any(), eq(Collections.singletonList("first-tag"))))
.thenThrow(new MediaWikiApiErrorException("badtags", "The tag 'first-tag' cannot be manually applied"));
when(editor.editEntityDocument(any(), anyBoolean(), any(), eq(Collections.singletonList("second-tag"))))
.thenReturn(new EditingResult(12345L));

EditBatchProcessor processor = new EditBatchProcessor(fetcher, editor, connection, batch, library, summary, maxlag,
Arrays.asList("first-tag", "second-tag"), 50, 60);
assertEquals(1, processor.remainingEdits());
processor.performEdit();
assertEquals(0, processor.remainingEdits());

verify(editor, times(1)).editEntityDocument(any(), anyBoolean(), any(), eq(Collections.singletonList("first-tag")));
verify(editor, times(1)).editEntityDocument(any(), anyBoolean(), any(), eq(Collections.singletonList("second-tag")));
}

@Test
public void testFallbackToNoTag() throws Exception {
List<EntityEdit> batch = new ArrayList<>();
batch.add(new ItemEditBuilder(TestingData.existingId)
.addAlias(Datamodel.makeMonolingualTextValue("my new alias", "en"))
.build());
ItemDocument existingItem = ItemDocumentBuilder.forItemId(TestingData.existingId)
.withLabel(Datamodel.makeMonolingualTextValue("pomme", "fr"))
.withDescription(Datamodel.makeMonolingualTextValue("fruit délicieux", "fr")).build();
when(fetcher.getEntityDocuments(Collections.singletonList(TestingData.existingId.getId())))
.thenReturn(Collections.singletonMap(TestingData.existingId.getId(), existingItem));

when(editor.editEntityDocument(any(), anyBoolean(), any(), eq(Collections.singletonList("first-tag"))))
.thenThrow(new MediaWikiApiErrorException("badtags", "The tag 'first-tag' cannot be manually applied"));
when(editor.editEntityDocument(any(), anyBoolean(), any(), eq(Collections.emptyList())))
.thenReturn(new EditingResult(12345L));

EditBatchProcessor processor = new EditBatchProcessor(fetcher, editor, connection, batch, library, summary, maxlag,
Arrays.asList("first-tag"), 50, 60);
assertEquals(1, processor.remainingEdits());
processor.performEdit();
assertEquals(0, processor.remainingEdits());

verify(editor, times(1)).editEntityDocument(any(), anyBoolean(), any(), eq(Collections.singletonList("first-tag")));
verify(editor, times(1)).editEntityDocument(any(), anyBoolean(), any(), eq(Collections.emptyList()));
}

@Test
public void testDeletedItem() throws IOException, MediaWikiApiErrorException, InterruptedException {
String id = "Q389";
Expand Down
Expand Up @@ -27,6 +27,8 @@
import static org.testng.Assert.assertEquals;

import java.io.LineNumberReader;
import java.util.Arrays;
import java.util.List;

import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
Expand Down Expand Up @@ -63,6 +65,15 @@ public void testConstructor() {
new PerformWikibaseEditsOperation(EngineConfig.reconstruct("{}"), "", 5, "", 60, "tag");
}

@Test
public void testGetTagCandidates() {
PerformWikibaseEditsOperation operation = new PerformWikibaseEditsOperation(
EngineConfig.reconstruct("{}"), "my summary", 5, "", 60, "openrefine-${version}");
List<String> candidates = operation.getTagCandidates("3.4");

assertEquals(candidates, Arrays.asList("openrefine-3.4", "openrefine"));
}

@Test
public void testLoadChange()
throws Exception {
Expand Down