Skip to content

Commit

Permalink
Separate out error reporting logic for testability
Browse files Browse the repository at this point in the history
  • Loading branch information
wetneb committed Apr 22, 2024
1 parent 4631125 commit c8e808d
Show file tree
Hide file tree
Showing 8 changed files with 283 additions and 45 deletions.
Expand Up @@ -30,6 +30,7 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.OptionalLong;
import java.util.Set;
import java.util.stream.Collectors;

Expand All @@ -39,6 +40,7 @@
import org.wikidata.wdtk.datamodel.interfaces.EntityIdValue;
import org.wikidata.wdtk.datamodel.interfaces.EntityUpdate;
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 @@ -158,7 +160,8 @@ public EditResult performEdit()
batchCursor++;
return new EditResult(update.getContributingRowIds(),
"rewrite-failed",
"Failed to rewrite update on entity " + update.getEntityId() + ". Missing entity: " + e.getMissingEntity());
"Failed to rewrite update on entity " + update.getEntityId() + ". Missing entity: " + e.getMissingEntity(),
OptionalLong.empty());
}

// Pick a tag to apply to the edits
Expand All @@ -167,6 +170,7 @@ public EditResult performEdit()
}
List<String> tags = currentTag == null ? Collections.emptyList() : Collections.singletonList(currentTag);

OptionalLong lastRevisionId = OptionalLong.empty();
try {
if (update.isNew()) {
// New entities
Expand Down Expand Up @@ -195,15 +199,17 @@ public EditResult performEdit()
}

if (entityUpdate != null && !entityUpdate.isEmpty()) { // skip updates which do not change anything
editor.editEntityDocument(entityUpdate, false, summary, tags);
EditingResult result = editor.editEntityDocument(entityUpdate, false, summary, tags);
lastRevisionId = result.getLastRevisionId();
}
// custom code for handling our custom updates to mediainfo, which cover editing more than Wikibase
if (entityUpdate instanceof FullMediaInfoUpdate) {
FullMediaInfoUpdate fullMediaInfoUpdate = (FullMediaInfoUpdate) entityUpdate;
if (fullMediaInfoUpdate.isOverridingWikitext() && fullMediaInfoUpdate.getWikitext() != null) {
MediaFileUtils mediaFileUtils = new MediaFileUtils(connection);
long pageId = Long.parseLong(fullMediaInfoUpdate.getEntityId().getId().substring(1));
mediaFileUtils.editPage(pageId, fullMediaInfoUpdate.getWikitext(), summary, tags);
long revisionId = mediaFileUtils.editPage(pageId, fullMediaInfoUpdate.getWikitext(), summary, tags);
lastRevisionId = OptionalLong.of(revisionId);
} else {
// manually purge the wikitext page associated with this mediainfo
MediaFileUtils mediaFileUtils = new MediaFileUtils(connection);
Expand All @@ -218,27 +224,30 @@ public EditResult performEdit()
return performEdit();
} else {
batchCursor++;
return new EditResult(update.getContributingRowIds(), e.getErrorCode(), e.getErrorMessage());
return new EditResult(update.getContributingRowIds(), e.getErrorCode(), e.getErrorMessage(), OptionalLong.empty());
}
} catch (IOException e) {
logger.warn("IO error while editing: " + e.getMessage());
}

batchCursor++;
return new EditResult(update.getContributingRowIds(), null, null);
return new EditResult(update.getContributingRowIds(), null, null, lastRevisionId);
}

public static class EditResult {

private final Set<Integer> correspondingRowIds;
private final String errorCode;
private final String errorMessage;
private final OptionalLong lastRevisionId;

public EditResult(Set<Integer> correspondingRowIds,
String errorCode, String errorMessage) {
String errorCode, String errorMessage,
OptionalLong lastRevisionId) {
this.correspondingRowIds = correspondingRowIds;
this.errorCode = errorCode;
this.errorMessage = errorMessage;
this.lastRevisionId = lastRevisionId;
}

public Set<Integer> getCorrespondingRowIds() {
Expand All @@ -252,6 +261,10 @@ public String getErrorCode() {
public String getErrorMessage() {
return errorMessage;
}

public OptionalLong getLastRevisionId() {
return lastRevisionId;
}
}

/**
Expand Down
@@ -0,0 +1,78 @@

package org.openrefine.wikibase.editing;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

import com.google.refine.expr.EvalError;
import com.google.refine.model.Cell;
import com.google.refine.model.changes.CellAtRow;

import org.openrefine.wikibase.editing.EditBatchProcessor.EditResult;

/**
* Formats the results of Wikibase edits into cells to be included in the project's grid.
*/
public class EditResultsFormatter {

private final String baseUrl;
private final Map<Integer, String> rowIdToError = new HashMap<>();
private final Map<Integer, String> rowIdToEditLink = new HashMap<>();

/**
* Constructor.
*
* @param mediaWikiApiEndpoint
* full API endpoint, ending with .../w/api.php
*/
public EditResultsFormatter(String mediaWikiApiEndpoint) {
this.baseUrl = mediaWikiApiEndpoint.substring(0, mediaWikiApiEndpoint.length() - "w/api.php".length());
}

/**
* Log another edit result for further formatting.
*/
public void add(EditResult result) {
if (!result.getCorrespondingRowIds().isEmpty()) {
int firstRowId = result.getCorrespondingRowIds().stream().min(Comparator.naturalOrder()).get();
if (result.getErrorMessage() != null && result.getErrorCode() != null) {
String error = String.format("[%s] %s", result.getErrorCode(), result.getErrorMessage());
String existingError = rowIdToError.get(firstRowId);
if (existingError == null) {
rowIdToError.put(firstRowId, error);
} else {
rowIdToError.put(firstRowId, existingError + "; " + error);
}
} else if (result.getLastRevisionId().isPresent()) {
String revisionLink = baseUrl + "w/index.php?diff=prev&oldid=" + result.getLastRevisionId().getAsLong();
String existingLinks = rowIdToEditLink.get(firstRowId);
if (existingLinks == null) {
rowIdToEditLink.put(firstRowId, revisionLink);
} else {
rowIdToEditLink.put(firstRowId, existingLinks + " " + revisionLink);
}
}
}
}

/**
* Generate a list of cells on specific rows representing the editing results
*/
public List<CellAtRow> toCells() {
List<CellAtRow> cells = new ArrayList<>();
for (Entry<Integer, String> errorCell : rowIdToError.entrySet()) {
cells.add(new CellAtRow(errorCell.getKey(), new Cell(new EvalError(errorCell.getValue()), null)));
}
for (Entry<Integer, String> editLinkCell : rowIdToEditLink.entrySet()) {
int rowId = editLinkCell.getKey();
if (rowIdToError.get(rowId) == null) {
cells.add(new CellAtRow(rowId, new Cell(editLinkCell.getValue(), null)));
}
}
return cells;
}
}
Expand Up @@ -146,12 +146,13 @@ public MediaUploadResponse uploadRemoteFile(URL url, String fileName, String wik
* the edit summary
* @param tags
* any tags that should be applied to the edit
* @return the revision id created by the edit
* @throws IOException
* if a network error happened
* @throws MediaWikiApiErrorException
* if the editing failed for some reason, after a few retries
*/
public void editPage(long pageId, String wikitext, String summary, List<String> tags) throws IOException, MediaWikiApiErrorException {
public long editPage(long pageId, String wikitext, String summary, List<String> tags) throws IOException, MediaWikiApiErrorException {
Map<String, String> parameters = new HashMap<>();
parameters.put("action", "edit");
parameters.put("tags", String.join("|", tags));
Expand All @@ -165,8 +166,12 @@ public void editPage(long pageId, String wikitext, String summary, List<String>
MediaWikiApiErrorException lastException = null;
while (retries > 0) {
try {
apiConnection.sendJsonRequest("POST", parameters);
return;
JsonNode response = apiConnection.sendJsonRequest("POST", parameters);
if (response.has("edit") && response.get("edit").has("newrevid")) {
return response.get("edit").get("newrevid").asLong(0L);
} else {
throw new IllegalStateException("Could not find the revision id in MediaWiki's response");
}
} catch (MediaWikiApiErrorException e) {
lastException = e;
}
Expand Down
Expand Up @@ -27,14 +27,9 @@
import java.io.IOException;
import java.io.LineNumberReader;
import java.io.Writer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
import java.util.Random;
import java.util.regex.Matcher;
Expand All @@ -58,7 +53,6 @@
import com.google.refine.RefineServlet;
import com.google.refine.browsing.Engine;
import com.google.refine.browsing.EngineConfig;
import com.google.refine.expr.EvalError;
import com.google.refine.history.Change;
import com.google.refine.history.HistoryEntry;
import com.google.refine.model.Cell;
Expand All @@ -77,6 +71,7 @@
import org.openrefine.wikibase.commands.ConnectionManager;
import org.openrefine.wikibase.editing.EditBatchProcessor;
import org.openrefine.wikibase.editing.EditBatchProcessor.EditResult;
import org.openrefine.wikibase.editing.EditResultsFormatter;
import org.openrefine.wikibase.editing.NewEntityLibrary;
import org.openrefine.wikibase.manifests.Manifest;
import org.openrefine.wikibase.schema.WikibaseSchema;
Expand Down Expand Up @@ -300,23 +295,14 @@ public void run() {
NewEntityLibrary newEntityLibrary = new NewEntityLibrary();
EditBatchProcessor processor = new EditBatchProcessor(fetcher, editor, connection, entityDocuments, newEntityLibrary, summary,
maxlag, getTagCandidates(RefineServlet.VERSION), 50, maxEditsPerMinute);
Map<Integer, String> rowIdToError = new HashMap<>();
EditResultsFormatter resultsFormatter = new EditResultsFormatter(mediaWikiApiEndpoint);

// Perform edits
logger.info("Performing edits");
while (processor.remainingEdits() > 0) {
try {
EditResult result = processor.performEdit();
if (result.getErrorMessage() != null && result.getErrorCode() != null && !result.getCorrespondingRowIds().isEmpty()) {
String error = String.format("[%s] %s", result.getErrorCode(), result.getErrorMessage());
int firstRowId = result.getCorrespondingRowIds().stream().min(Comparator.naturalOrder()).get();
String existingError = rowIdToError.get(firstRowId);
if (existingError == null) {
rowIdToError.put(firstRowId, error);
} else {
rowIdToError.put(firstRowId, existingError + "; " + error);
}
}
resultsFormatter.add(result);
} catch (InterruptedException e) {
_canceled = true;
}
Expand All @@ -334,10 +320,7 @@ public void run() {
Column resultsColumn = _project.columnModel.getColumnByName(resultsColumnName);
int resultsCellIndex = resultsColumn != null ? resultsColumn.getCellIndex() : -1;

List<CellAtRow> cells = new ArrayList<>();
for (Entry<Integer, String> errorCell : rowIdToError.entrySet()) {
cells.add(new CellAtRow(errorCell.getKey(), new Cell(new EvalError(errorCell.getValue()), null)));
}
List<CellAtRow> cells = resultsFormatter.toCells();

if (resultsCellIndex != -1) {
// column already exists, we overwrite cells where we made edits
Expand Down

0 comments on commit c8e808d

Please sign in to comment.