This is an automated email from the ASF dual-hosted git repository. kuyu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-gobblin.git
The following commit(s) were added to refs/heads/master by this push: new d208c03 [GOBBLIN-1218] Old props deserialization2 d208c03 is described below commit d208c0336a7d4e8e91b723ee7fc3bef6326aec21 Author: Arjun <ab...@linkedin.com> AuthorDate: Tue Jul 21 17:17:30 2020 -0700 [GOBBLIN-1218] Old props deserialization2 Dear Gobblin maintainers, Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below! jack-moseley please review ### JIRA - [x] My PR addresses the following [Gobblin JIRA] (https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR" - https://issues.apache.org/jira/browse/GOBBLIN-1218 ### Description - [x] Here are some details about my PR, including screenshots (if applicable): 1) attempt to deserialize flow config properties using old method if deserialization using new method fails 2) some configs can have special characters like dot (.) , these properties have to be escaped in mysql query ### Tests - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: added a unit test ### Commits - [x] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git- commit/)": 1. Subject is separated from body by a blank line 2. Subject is limited to 50 characters 3. Subject does not end with a period 4. Subject uses the imperative mood ("add", not "adding") 5. Body wraps at 72 characters 6. Body explains "what" and "why", not "how" deserliaze props serialized with old way escape special characters in mysql query Closes #3066 from arjun4084346/oldPropsDeserialization2 --- .../gobblin/runtime/spec_serde/FlowSpecDeserializer.java | 16 +++++++++++++++- .../gobblin/runtime/spec_store/MysqlSpecStore.java | 4 ++-- .../gobblin/runtime/spec_store/MysqlSpecStoreTest.java | 6 ++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_serde/FlowSpecDeserializer.java b/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_serde/FlowSpecDeserializer.java index 00cf513..bc8c59b 100644 --- a/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_serde/FlowSpecDeserializer.java +++ b/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_serde/FlowSpecDeserializer.java @@ -17,6 +17,8 @@ package org.apache.gobblin.runtime.spec_serde; +import java.io.IOException; +import java.io.StringReader; import java.lang.reflect.Type; import java.net.URI; import java.net.URISyntaxException; @@ -48,7 +50,19 @@ public class FlowSpecDeserializer implements JsonDeserializer<FlowSpec> { String description = jsonObject.get(FlowSpecSerializer.FLOW_SPEC_DESCRIPTION_KEY).getAsString(); Config config = ConfigFactory.parseString(jsonObject.get(FlowSpecSerializer.FLOW_SPEC_CONFIG_KEY).getAsString()); - Properties properties = context.deserialize(jsonObject.get(FlowSpecSerializer.FLOW_SPEC_CONFIG_AS_PROPERTIES_KEY), Properties.class); + Properties properties; + + try { + properties = context.deserialize(jsonObject.get(FlowSpecSerializer.FLOW_SPEC_CONFIG_AS_PROPERTIES_KEY), Properties.class); + } catch (JsonParseException e) { + // for backward compatibility + properties = new Properties(); + try { + properties.load(new StringReader(jsonObject.get(FlowSpecSerializer.FLOW_SPEC_CONFIG_AS_PROPERTIES_KEY).getAsString())); + } catch (IOException ioe) { + throw new JsonParseException(e); + } + } Set<URI> templateURIs = new HashSet<>(); try { diff --git a/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStore.java b/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStore.java index ff6bc85..a3b5848 100644 --- a/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStore.java +++ b/gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStore.java @@ -339,9 +339,9 @@ public class MysqlSpecStore extends InstrumentedSpecStore { log.error("Incorrect flow config search query"); continue; } - conditions.add("spec_json->'$.configAsProperties." + keyValue[0] + "' like " + "'%" + keyValue[1] + "%'"); + conditions.add("spec_json->'$.configAsProperties.\"" + keyValue[0] + "\"' like " + "'%" + keyValue[1] + "%'"); } else { - conditions.add("spec_json->'$.configAsProperties." + property + "' is not null"); + conditions.add("spec_json->'$.configAsProperties.\"" + property + "\"' is not null"); } } } diff --git a/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStoreTest.java b/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStoreTest.java index ec6a14e..b6d720b 100644 --- a/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStoreTest.java +++ b/gobblin-runtime/src/test/java/org/apache/gobblin/runtime/spec_store/MysqlSpecStoreTest.java @@ -86,6 +86,7 @@ public class MysqlSpecStoreTest { .withConfig(ConfigBuilder.create() .addPrimitive("key", "value") .addPrimitive("key3", "value3") + .addPrimitive("config.with.dot", "value4") .addPrimitive(FLOW_SOURCE_IDENTIFIER_KEY, "source") .addPrimitive(FLOW_DESTINATION_IDENTIFIER_KEY, "destination") .addPrimitive(ConfigurationKeys.FLOW_GROUP_KEY, "fg1") @@ -174,6 +175,11 @@ public class MysqlSpecStoreTest { Assert.assertEquals(specs.size(), 2); Assert.assertTrue(specs.contains(this.flowSpec1)); Assert.assertTrue(specs.contains(this.flowSpec2)); + + flowSpecSearchObject = FlowSpecSearchObject.builder().propertyFilter("config.with.dot=value4").build(); + specs = this.specStore.getSpecs(flowSpecSearchObject); + Assert.assertEquals(specs.size(), 1); + Assert.assertTrue(specs.contains(this.flowSpec1)); } @Test (dependsOnMethods = "testGetSpec")