s434730/RandomSec
3
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix JSON history corruption.

Browse Source 
Also adds new logic to preserve the JSON representation of unknown operations,
to protect from version downgrading or removal of extensions.

Closes #1990.
This commit is contained in:
Antonin Delpeuch 2019-04-18 20:03:08 +01:00
parent b1e6a5306a
commit 0332be312f
6 changed files with 155 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
main/src/com/google/refine/io/FileHistoryEntryManager.java
View File

@ -64,7 +64,11 @@ public class FileHistoryEntryManager implements HistoryEntryManager{
@Override
public void save(HistoryEntry historyEntry, Writer writer, Properties options) {
try {
ParsingUtilities.defaultWriter.writeValue(writer, historyEntry);
if("save".equals(options.getProperty("mode"))) {
ParsingUtilities.saveWriter.writeValue(writer, historyEntry);
} else {
ParsingUtilities.defaultWriter.writeValue(writer, historyEntry);
}
} catch (IOException e) {
e.printStackTrace();
}

3
main/src/com/google/refine/model/AbstractOperation.java
View File

@ -51,7 +51,8 @@ import com.google.refine.process.QuickHistoryEntryProcess;
@JsonTypeInfo(
use=JsonTypeInfo.Id.CUSTOM,
include=JsonTypeInfo.As.PROPERTY,
property="op")
property="op",
visible=true) // for UnknownOperation, which needs to read its own id
@JsonTypeIdResolver(OperationResolver.class)
abstract public class AbstractOperation {
public Process createProcess(Project project, Properties options) throws Exception {

13
main/src/com/google/refine/operations/OperationResolver.java
View File

@ -51,11 +51,20 @@ public class OperationResolver extends TypeIdResolverBase {
@Override
public String idFromValueAndType(Object instance, Class<?> type) {
return OperationRegistry.s_opClassToName.get(type);
String id = OperationRegistry.s_opClassToName.get(type);
if (id != null) {
return id;
} else { // this happens for an UnknownOperation
return ((AbstractOperation) instance).getOperationId();
}
}
@Override
public JavaType typeFromId(DatabindContext context, String id) throws IOException {
return factory.constructSimpleType(OperationRegistry.resolveOperationId(id), new JavaType[0]);
Class<? extends AbstractOperation> opClass = OperationRegistry.resolveOperationId(id);
if (opClass == null) {
opClass = UnknownOperation.class;
}
return factory.constructSimpleType(opClass, new JavaType[0]);
}
}

62
main/src/com/google/refine/operations/UnknownOperation.java Normal file
View File

@ -0,0 +1,62 @@
package com.google.refine.operations;
import java.util.HashMap;
import java.util.Map;
import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.refine.model.AbstractOperation;
import com.google.refine.model.Project;
/**
* An operation that is unknown to the current OpenRefine
* instance, but might be interpretable by another instance
* (for instance, a later version of OpenRefine, or using
* an extension).
*
* This class holds the JSON serialization of the operation,
* in the interest of being able to serialize it later, hence
* avoiding to discard it and lose metadata.
*
* @author Antonin Delpeuch
*
*/
public class UnknownOperation extends AbstractOperation {
// Map storing the JSON serialization of the operation in an agnostic way
private Map<String, Object> properties;
// Operation code and description stored separately
private String opCode;
private String description;
@JsonCreator
public UnknownOperation(
@JsonProperty("op") String opCode,
@JsonProperty("description") String description) {
properties = new HashMap<>();
this.opCode = opCode;
this.description = description;
}
@JsonAnySetter
public void setAttribute(String key, Object value) {
properties.put(key, value);
}
@JsonAnyGetter
public Map<String,Object> getAttributes() {
return properties;
}
@JsonProperty("op")
public String getOperationId() {
return opCode;
}
protected String getBriefDescription(Project project) {
return description;
}
}

40
main/tests/server/src/com/google/refine/tests/history/FileHistoryEntryManagerTests.java Normal file
View File

@ -0,0 +1,40 @@
package com.google.refine.tests.history;
import static org.mockito.Mockito.mock;
import java.io.IOException;
import java.io.StringWriter;
import java.util.Properties;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import com.google.refine.history.HistoryEntry;
import com.google.refine.io.FileHistoryEntryManager;
import com.google.refine.model.Project;
import com.google.refine.operations.OperationRegistry;
import com.google.refine.operations.column.ColumnAdditionOperation;
import com.google.refine.tests.RefineTest;
import com.google.refine.tests.util.TestUtils;
public class FileHistoryEntryManagerTests extends RefineTest {
Project project;
FileHistoryEntryManager sut = new FileHistoryEntryManager();
@BeforeMethod
public void setUp() {
project = mock(Project.class);
OperationRegistry.registerOperation(getCoreModule(), "column-addition", ColumnAdditionOperation.class);
}
@Test
public void testWriteHistoryEntry() throws IOException {
StringWriter writer = new StringWriter();
HistoryEntry historyEntry = HistoryEntry.load(project, HistoryEntryTests.fullJson);
Properties options = new Properties();
options.setProperty("mode", "save");
sut.save(historyEntry, writer, options);
TestUtils.equalAsJson(HistoryEntryTests.fullJson, writer.toString());
}
}

50
main/tests/server/src/com/google/refine/tests/history/HistoryEntryTests.java
View File

@ -28,6 +28,8 @@ package com.google.refine.tests.history;
import static org.mockito.Mockito.mock;
import java.io.IOException;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
@ -40,6 +42,30 @@ import com.google.refine.tests.RefineTest;
import com.google.refine.tests.util.TestUtils;
public class HistoryEntryTests extends RefineTest {
public static final String fullJson = "{"
+ "\"id\":1533633623158,"
+ "\"description\":\"Create new column uri based on column country by filling 269 rows with grel:\\\"https://www.wikidata.org/wiki/\\\"+cell.recon.match.id\","
+ "\"time\":\"2018-08-07T09:06:37Z\","
+ "\"operation\":{\"op\":\"core/column-addition\","
+ " \"description\":\"Create column uri at index 2 based on column country using expression grel:\\\"https://www.wikidata.org/wiki/\\\"+cell.recon.match.id\","
+ " \"engineConfig\":{\"mode\":\"row-based\",\"facets\":[]},"
+ " \"newColumnName\":\"uri\","
+ " \"columnInsertIndex\":2,"
+ " \"baseColumnName\":\"country\","
+ " \"expression\":\"grel:\\\"https://www.wikidata.org/wiki/\\\"+cell.recon.match.id\","
+ " \"onError\":\"set-to-blank\"}"
+ "}";
public static final String unknownOperationJson = "{"
+ "\"id\":1533633623158,"
+ "\"description\":\"some mysterious operation\","
+ "\"time\":\"2018-08-07T09:06:37Z\","
+ "\"operation\":{\"op\":\"someextension/unknown-operation\","
+ " \"description\":\"some mysterious operation\","
+ " \"some_parameter\":234\n"
+ "}\n"
+ "}";
Project project;
@ -63,26 +89,20 @@ public class HistoryEntryTests extends RefineTest {
@Test
public void serializeHistoryEntryWithOperation() throws Exception {
String json = "{"
+ "\"id\":1533633623158,"
+ "\"description\":\"Create new column uri based on column country by filling 269 rows with grel:\\\"https://www.wikidata.org/wiki/\\\"+cell.recon.match.id\","
+ "\"time\":\"2018-08-07T09:06:37Z\","
+ "\"operation\":{\"op\":\"core/column-addition\","
+ " \"description\":\"Create column uri at index 2 based on column country using expression grel:\\\"https://www.wikidata.org/wiki/\\\"+cell.recon.match.id\","
+ " \"engineConfig\":{\"mode\":\"row-based\",\"facets\":[]},"
+ " \"newColumnName\":\"uri\","
+ " \"columnInsertIndex\":2,"
+ " \"baseColumnName\":\"country\","
+ " \"expression\":\"grel:\\\"https://www.wikidata.org/wiki/\\\"+cell.recon.match.id\","
+ " \"onError\":\"set-to-blank\"}"
+ "}";
String jsonSimple = "{"
+ "\"id\":1533633623158,"
+ "\"description\":\"Create new column uri based on column country by filling 269 rows with grel:\\\"https://www.wikidata.org/wiki/\\\"+cell.recon.match.id\","
+ "\"time\":\"2018-08-07T09:06:37Z\"}";
HistoryEntry historyEntry = HistoryEntry.load(project, json);
HistoryEntry historyEntry = HistoryEntry.load(project, fullJson);
TestUtils.isSerializedTo(historyEntry, jsonSimple, false);
TestUtils.isSerializedTo(historyEntry, json, true);
TestUtils.isSerializedTo(historyEntry, fullJson, true);
}
@Test
public void deserializeUnknownOperation() throws IOException {
// Unknown operations are serialized back as they were parsed
HistoryEntry entry = HistoryEntry.load(project, unknownOperationJson);
TestUtils.isSerializedTo(entry, unknownOperationJson, true);
}
}