From 0dae0811b070b57b67a909334573c640e96bc53e Mon Sep 17 00:00:00 2001 From: Antonin Delpeuch Date: Sun, 21 Oct 2018 16:31:58 +0100 Subject: [PATCH] Jackson deserialization for ReconConfig --- .../commands/recon/ReconcileCommand.java | 8 +- .../model/changes/DataExtensionChange.java | 2 +- .../refine/model/changes/ReconChange.java | 8 +- .../refine/model/recon/ReconConfig.java | 53 +++++++------ .../model/recon/ReconConfigResolver.java | 34 +++++++++ .../model/recon/StandardReconConfig.java | 76 ++++++++----------- .../operations/recon/ReconOperation.java | 2 +- .../refine/tests/model/ReconTypeTest.java | 3 +- .../model/recon/StandardReconConfigTests.java | 13 +++- 9 files changed, 110 insertions(+), 89 deletions(-) create mode 100644 main/src/com/google/refine/model/recon/ReconConfigResolver.java diff --git a/main/src/com/google/refine/commands/recon/ReconcileCommand.java b/main/src/com/google/refine/commands/recon/ReconcileCommand.java index 7861e0db6..21866ed80 100644 --- a/main/src/com/google/refine/commands/recon/ReconcileCommand.java +++ b/main/src/com/google/refine/commands/recon/ReconcileCommand.java @@ -35,9 +35,6 @@ package com.google.refine.commands.recon; import javax.servlet.http.HttpServletRequest; -import org.json.JSONObject; -import org.json.JSONTokener; - import com.google.refine.browsing.EngineConfig; import com.google.refine.commands.EngineDependentCommand; import com.google.refine.model.AbstractOperation; @@ -54,9 +51,6 @@ public class ReconcileCommand extends EngineDependentCommand { String columnName = request.getParameter("columnName"); String configString = request.getParameter("config"); - JSONTokener t = new JSONTokener(configString); - JSONObject config = (JSONObject) t.nextValue(); - - return new ReconOperation(engineConfig, columnName, ReconConfig.reconstruct(config)); + return new ReconOperation(engineConfig, columnName, ReconConfig.reconstruct(configString)); } } diff --git a/main/src/com/google/refine/model/changes/DataExtensionChange.java b/main/src/com/google/refine/model/changes/DataExtensionChange.java index ce3bccc96..1bb2a35c7 100644 --- a/main/src/com/google/refine/model/changes/DataExtensionChange.java +++ b/main/src/com/google/refine/model/changes/DataExtensionChange.java @@ -393,7 +393,7 @@ public class DataExtensionChange implements Change { if (line == null || line.length() == 0) { columnTypes.add(null); } else { - columnTypes.add(ReconType.load(ParsingUtilities.evaluateJsonStringToObject(line))); + columnTypes.add(ReconType.load(line)); } } } else if ("dataExtensionCount".equals(field)) { diff --git a/main/src/com/google/refine/model/changes/ReconChange.java b/main/src/com/google/refine/model/changes/ReconChange.java index 9b5508c5d..76440cdbc 100644 --- a/main/src/com/google/refine/model/changes/ReconChange.java +++ b/main/src/com/google/refine/model/changes/ReconChange.java @@ -174,19 +174,19 @@ public class ReconChange extends MassCellChange { if ("newReconConfig".equals(field)) { if (value.length() > 0) { - newReconConfig = ReconConfig.reconstruct(ParsingUtilities.evaluateJsonStringToObject(value)); + newReconConfig = ReconConfig.reconstruct(value); } } else if ("newReconStats".equals(field)) { if (value.length() > 0) { - newReconStats = ReconStats.load(ParsingUtilities.evaluateJsonStringToObject(value)); + newReconStats = ParsingUtilities.mapper.readValue(value, ReconStats.class); } } else if ("oldReconConfig".equals(field)) { if (value.length() > 0) { - oldReconConfig = ReconConfig.reconstruct(ParsingUtilities.evaluateJsonStringToObject(value)); + oldReconConfig = ReconConfig.reconstruct(value); } } else if ("oldReconStats".equals(field)) { if (value.length() > 0) { - oldReconStats = ReconStats.load(ParsingUtilities.evaluateJsonStringToObject(value)); + oldReconStats = ParsingUtilities.mapper.readValue(value, ReconStats.class); } } else if ("commonColumnName".equals(field)) { commonColumnName = value; diff --git a/main/src/com/google/refine/model/recon/ReconConfig.java b/main/src/com/google/refine/model/recon/ReconConfig.java index 063cc5e13..aa3459a89 100644 --- a/main/src/com/google/refine/model/recon/ReconConfig.java +++ b/main/src/com/google/refine/model/recon/ReconConfig.java @@ -35,17 +35,17 @@ package com.google.refine.model.recon; import java.io.IOException; import java.io.Writer; -import java.lang.reflect.Method; import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; -import org.json.JSONObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.databind.annotation.JsonTypeIdResolver; import com.google.refine.model.Cell; import com.google.refine.model.Project; @@ -55,6 +55,11 @@ import com.google.refine.util.ParsingUtilities; import edu.mit.simile.butterfly.ButterflyModule; +@JsonTypeInfo( + use=JsonTypeInfo.Id.CUSTOM, + include=JsonTypeInfo.As.PROPERTY, + property="mode") +@JsonTypeIdResolver(ReconConfigResolver.class) abstract public class ReconConfig { final static protected Logger LOGGER = LoggerFactory.getLogger("recon-config"); @@ -77,35 +82,29 @@ abstract public class ReconConfig { classes.add(klass); } - static public ReconConfig reconstruct(JSONObject obj) throws Exception { - try { - String mode = obj.getString("mode"); - - // Backward compatibility - if ("extend".equals(mode) || "strict".equals(mode)) { - mode = "freebase/" + mode; - } else if ("heuristic".equals(mode)) { - mode = "core/standard-service"; // legacy - } else if (!mode.contains("/")) { - mode = "core/" + mode; - } - - // TODO: This can fail silently if the Freebase extension is not installed. - List> classes = s_opNameToClass.get(mode); - if (classes != null && classes.size() > 0) { - Class klass = classes.get(classes.size() - 1); - - Method reconstruct = klass.getMethod("reconstruct", JSONObject.class); - if (reconstruct != null) { - return (ReconConfig) reconstruct.invoke(null, obj); - } - } - } catch (Exception e) { - LOGGER.error("Reconstruct failed",e); + static public Class getClassFromMode(String mode) { + // Backward compatibility + if ("extend".equals(mode) || "strict".equals(mode)) { + mode = "freebase/" + mode; + } else if ("heuristic".equals(mode)) { + mode = "core/standard-service"; // legacy + } else if (!mode.contains("/")) { + mode = "core/" + mode; + } + + // TODO: This can fail silently if the Freebase extension is not installed. + List> classes = s_opNameToClass.get(mode); + System.out.println(classes); + if (classes != null && classes.size() > 0) { + return classes.get(classes.size() - 1); } return null; } + static public ReconConfig reconstruct(String json) throws Exception { + return ParsingUtilities.mapper.readValue(json, ReconConfig.class); + } + abstract public int getBatchSize(); abstract public String getBriefDescription(Project project, String columnName); diff --git a/main/src/com/google/refine/model/recon/ReconConfigResolver.java b/main/src/com/google/refine/model/recon/ReconConfigResolver.java new file mode 100644 index 000000000..ed8aaffef --- /dev/null +++ b/main/src/com/google/refine/model/recon/ReconConfigResolver.java @@ -0,0 +1,34 @@ +package com.google.refine.model.recon; + +import java.io.IOException; + +import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; +import com.fasterxml.jackson.databind.DatabindContext; +import com.fasterxml.jackson.databind.JavaType; +import com.fasterxml.jackson.databind.jsontype.impl.TypeIdResolverBase; +import com.fasterxml.jackson.databind.type.TypeFactory; + +public class ReconConfigResolver extends TypeIdResolverBase { + + protected TypeFactory factory = TypeFactory.defaultInstance(); + + @Override + public Id getMechanism() { + return Id.NAME; + } + + @Override + public String idFromValue(Object instance) { + return ((ReconConfig)instance).getMode(); + } + + @Override + public String idFromValueAndType(Object instance, Class type) { + return ReconConfig.s_opClassToName.get(type); + } + + @Override + public JavaType typeFromId(DatabindContext context, String id) throws IOException { + return factory.constructSimpleType(ReconConfig.getClassFromMode(id), new JavaType[0]); + } +} diff --git a/main/src/com/google/refine/model/recon/StandardReconConfig.java b/main/src/com/google/refine/model/recon/StandardReconConfig.java index 692e0958e..a8b568feb 100644 --- a/main/src/com/google/refine/model/recon/StandardReconConfig.java +++ b/main/src/com/google/refine/model/recon/StandardReconConfig.java @@ -34,11 +34,11 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package com.google.refine.model.recon; import java.io.DataOutputStream; +import java.io.IOException; import java.io.InputStream; import java.io.StringWriter; import java.net.HttpURLConnection; import java.net.URL; -import java.time.OffsetDateTime; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -54,6 +54,7 @@ import org.json.JSONObject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonInclude.Include; @@ -82,56 +83,22 @@ public class StandardReconConfig extends ReconConfig { @JsonProperty("propertyID") final public String propertyID; - public ColumnDetail(String columnName, String propertyName, String propertyID) { + @JsonCreator + public ColumnDetail( + @JsonProperty("column") + String columnName, + @JsonProperty("propertyName") + String propertyName, + @JsonProperty("propertyID") + String propertyID) { this.columnName = columnName; this.propertyName = propertyName; this.propertyID = propertyID; } } - static public ReconConfig reconstruct(JSONObject obj) throws Exception { - List columnDetails = null; - if (obj.has("columnDetails")) { - JSONArray columnDetailsA = obj.getJSONArray("columnDetails"); - int l = columnDetailsA.length(); - - columnDetails = new ArrayList(l); - for (int i = 0; i < l; i++) { - JSONObject o = columnDetailsA.getJSONObject(i); - - if (o.has("property")) { // legacy - JSONObject p = o.getJSONObject("property"); - columnDetails.add(new ColumnDetail( - o.getString("column"), - p.has("name") ? p.getString("name") : null, - p.has("id") ? p.getString("id") : null - )); - } else { - columnDetails.add(new ColumnDetail( - o.getString("column"), - o.has("propertyName") ? o.getString("propertyName") : null, - o.has("propertyID") ? o.getString("propertyID") : null - )); - } - } - } else { - columnDetails = new ArrayList(); - } - - JSONObject t = obj.has("type") && !obj.isNull("type") ? obj.getJSONObject("type") : null; - - int limit = obj.has("limit") && !obj.isNull("limit") ? obj.getInt("limit") : 0; - - return new StandardReconConfig( - obj.getString("service"), - obj.has("identifierSpace") ? obj.getString("identifierSpace") : null, - obj.has("schemaSpace") ? obj.getString("schemaSpace") : null, - t == null ? null : t.getString("id"), - t == null ? null : (t.has("name") ? t.getString("name") : null), - obj.getBoolean("autoMatch"), - columnDetails, - limit - ); + static public ReconConfig reconstruct(String json) throws IOException { + return ParsingUtilities.mapper.readValue(json, ReconConfig.class); } static protected class StandardReconJob extends ReconJob { @@ -162,6 +129,25 @@ public class StandardReconConfig extends ReconConfig { @JsonProperty("limit") final private int limit; + @JsonCreator + public StandardReconConfig( + @JsonProperty("service") + String service, + @JsonProperty("identifierSpace") + String identifierSpace, + @JsonProperty("schemaSpace") + String schemaSpace, + @JsonProperty("type") + ReconType type, + @JsonProperty("autoMatch") + boolean autoMatch, + @JsonProperty("columnDetails") + List columnDetails, + @JsonProperty("limit") + int limit) { + this(service, identifierSpace, schemaSpace, type.id, type.name, autoMatch, columnDetails, limit); + } + public StandardReconConfig( String service, String identifierSpace, diff --git a/main/src/com/google/refine/operations/recon/ReconOperation.java b/main/src/com/google/refine/operations/recon/ReconOperation.java index 976b1acb3..612b7d57e 100644 --- a/main/src/com/google/refine/operations/recon/ReconOperation.java +++ b/main/src/com/google/refine/operations/recon/ReconOperation.java @@ -81,7 +81,7 @@ public class ReconOperation extends EngineDependentOperation { return new ReconOperation( EngineConfig.reconstruct(engineConfig), obj.getString("columnName"), - ReconConfig.reconstruct(obj.getJSONObject("config")) + ReconConfig.reconstruct(obj.getJSONObject("config").toString()) ); } diff --git a/main/tests/server/src/com/google/refine/tests/model/ReconTypeTest.java b/main/tests/server/src/com/google/refine/tests/model/ReconTypeTest.java index 9e66b7ca8..e52dcd59d 100644 --- a/main/tests/server/src/com/google/refine/tests/model/ReconTypeTest.java +++ b/main/tests/server/src/com/google/refine/tests/model/ReconTypeTest.java @@ -1,7 +1,6 @@ package com.google.refine.tests.model; import org.json.JSONException; -import org.json.JSONObject; import org.testng.annotations.Test; import com.google.refine.model.ReconType; @@ -11,7 +10,7 @@ public class ReconTypeTest { @Test public void serializeReconType() throws JSONException, Exception { String json = "{\"id\":\"Q7540126\",\"name\":\"headquarters\"}"; - ReconType rt = ReconType.load(new JSONObject(json)); + ReconType rt = ReconType.load(json); TestUtils.isSerializedTo(rt, json); } } diff --git a/main/tests/server/src/com/google/refine/tests/model/recon/StandardReconConfigTests.java b/main/tests/server/src/com/google/refine/tests/model/recon/StandardReconConfigTests.java index 470f1ecfe..41e55d4c4 100644 --- a/main/tests/server/src/com/google/refine/tests/model/recon/StandardReconConfigTests.java +++ b/main/tests/server/src/com/google/refine/tests/model/recon/StandardReconConfigTests.java @@ -2,18 +2,27 @@ package com.google.refine.tests.model.recon; import java.util.ArrayList; -import org.json.JSONObject; import org.slf4j.LoggerFactory; import org.testng.Assert; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; import com.google.refine.model.recon.ReconConfig; import com.google.refine.model.recon.StandardReconConfig; +import com.google.refine.operations.OperationRegistry; +import com.google.refine.operations.recon.ReconOperation; import com.google.refine.tests.RefineTest; import com.google.refine.tests.util.TestUtils; public class StandardReconConfigTests extends RefineTest { + + @BeforeMethod + public void registerOperation() { + OperationRegistry.registerOperation(getCoreModule(), "recon", ReconOperation.class); + ReconConfig.registerReconConfig(getCoreModule(), "standard-service", StandardReconConfig.class); + } + @Override @BeforeTest @@ -74,7 +83,7 @@ public class StandardReconConfigTests extends RefineTest { " ],\n" + " \"limit\": 0\n" + " }"; - ReconConfig config = StandardReconConfig.reconstruct(new JSONObject(json)); + ReconConfig config = ReconConfig.reconstruct(json); TestUtils.isSerializedTo(config, json); } }