You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gobblin.apache.org by ku...@apache.org on 2020/07/22 00:18:17 UTC

[incubator-gobblin] branch master updated: [GOBBLIN-1218] Old props deserialization2

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")