You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by dk...@apache.org on 2018/11/16 19:17:26 UTC

[avro] branch master updated: [AVRO-1605] Remove deprecated API's that expose JsonNode

This is an automated email from the ASF dual-hosted git repository.

dkulp pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git


The following commit(s) were added to refs/heads/master by this push:
     new a7fb391  [AVRO-1605] Remove deprecated API's that expose JsonNode
a7fb391 is described below

commit a7fb39172ddec93e289127e1a58d7ea1e0b3a498
Author: Daniel Kulp <dk...@apache.org>
AuthorDate: Fri Nov 16 14:14:50 2018 -0500

    [AVRO-1605] Remove deprecated API's that expose JsonNode
---
 .../main/java/org/apache/avro/JsonProperties.java  | 73 ++++++++++------------
 .../src/main/java/org/apache/avro/Protocol.java    | 64 ++++++++++++-------
 .../avro/src/main/java/org/apache/avro/Schema.java | 14 ++---
 .../java/org/apache/avro/TestSchemaBuilder.java    | 37 +----------
 .../apache/avro/compiler/idl/SchemaResolver.java   |  4 +-
 .../java/org/apache/avro/mojo/IDLProtocolMojo.java | 29 ++++-----
 6 files changed, 98 insertions(+), 123 deletions(-)

diff --git a/lang/java/avro/src/main/java/org/apache/avro/JsonProperties.java b/lang/java/avro/src/main/java/org/apache/avro/JsonProperties.java
index 2fca58d..fc83bd5 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/JsonProperties.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/JsonProperties.java
@@ -153,6 +153,9 @@ public abstract class JsonProperties {
       }
       return r;
     }
+    public JsonNode put(String key,  JsonNode value) {
+      return putIfAbsent(key, value);
+    }
     public Set<Map.Entry<String, JsonNode>> entrySet() {
       return new AbstractSet<Map.Entry<String, JsonNode>>() {
         @Override
@@ -182,6 +185,21 @@ public abstract class JsonProperties {
   JsonProperties(Set<String> reserved) {
     this.reserved = reserved;
   }
+  JsonProperties(Set<String> reserved, Map<String,?> propMap) {
+    this.reserved = reserved;
+    for (Entry<String, ?> a : propMap.entrySet()) {
+      Object v = a.getValue();
+      JsonNode json = null;
+      if (v instanceof String) {
+        json = TextNode.valueOf((String)v);
+      } else if (v instanceof JsonNode){
+        json = (JsonNode)v;
+      } else {
+        json = JacksonUtils.toJsonNode(v);
+      }
+      props.put(a.getKey(), json);
+    }
+  }
 
   /**
    * Returns the value of the named, string-valued property in this schema.
@@ -196,8 +214,7 @@ public abstract class JsonProperties {
    * Returns the value of the named property in this schema.
    * Returns <tt>null</tt> if there is no property with that name.
    */
-  @Deprecated
-  public JsonNode getJsonProp(String name) {
+  private JsonNode getJsonProp(String name) {
     return props.get(name);
   }
 
@@ -221,6 +238,18 @@ public abstract class JsonProperties {
   public void addProp(String name, String value) {
     addProp(name, TextNode.valueOf(value));
   }
+  public void addProp(String name, Object value) {
+    if (value instanceof JsonNode) {
+      addProp(name, (JsonNode)value);
+    } else {
+      addProp(name, JacksonUtils.toJsonNode(value));
+    }
+  }
+  public void putAll(JsonProperties np) {
+    for (Map.Entry<? extends String, ? extends JsonNode> e : np.props.entrySet())
+      addProp(e.getKey(), e.getValue());
+  }
+
 
   /**
    * Adds a property with the given name <tt>name</tt> and
@@ -230,10 +259,8 @@ public abstract class JsonProperties {
    *
    * @param name The name of the property to add
    * @param value The value for the property to add
-   * @deprecated use {@link #addProp(String, Object)}
    */
-  @Deprecated
-  public void addProp(String name, JsonNode value) {
+  private void addProp(String name, JsonNode value) {
     if (reserved.contains(name))
       throw new AvroRuntimeException("Can't set reserved property: " + name);
 
@@ -245,13 +272,6 @@ public abstract class JsonProperties {
       throw new AvroRuntimeException("Can't overwrite property: " + name);
     }
   }
-  public void addProp(String name, Object value) {
-    addProp(name, JacksonUtils.toJsonNode(value));
-  }
-  public void putAll(JsonProperties np) {
-    for (Map.Entry<? extends String, ? extends JsonNode> e : np.props.entrySet())
-      addProp(e.getKey(), e.getValue());
-  }
 
 
   /**
@@ -260,41 +280,16 @@ public abstract class JsonProperties {
    * @see #getObjectProps()
    */
   public void addAllProps(JsonProperties properties) {
-    for (Entry<String, JsonNode> entry : properties.getJsonProps().entrySet())
+    for (Entry<String, JsonNode> entry : properties.props.entrySet())
       addProp(entry.getKey(), entry.getValue());
   }
 
-  /** Return the defined properties that have string values. */
-  @Deprecated public Map<String,String> getProps() {
-    Map<String,String> result = new LinkedHashMap<>();
-    for (Map.Entry<String,JsonNode> e : props.entrySet())
-      if (e.getValue().isTextual())
-        result.put(e.getKey(), e.getValue().textValue());
-    return result;
-  }
-
-  /** Convert a map of string-valued properties to Json properties. */
-  Map<String,JsonNode> jsonProps(Map<String,String> stringProps) {
-    Map<String,JsonNode> result = new LinkedHashMap<>();
-    for (Map.Entry<String,String> e : stringProps.entrySet())
-      result.put(e.getKey(), TextNode.valueOf(e.getValue()));
-    return result;
-  }
-
-  /**
-   * Return the defined properties as an unmodifieable Map.
-   * @deprecated use {@link #getObjectProps()}
-   */
-  Map<String,JsonNode> getJsonProps() {
-    return Collections.unmodifiableMap(props);
-  }
-
   /** Return the defined properties as an unmodifiable Map. */
   public Map<String,Object> getObjectProps() {
     Map<String,Object> result = new LinkedHashMap<>();
     for (Map.Entry<String,JsonNode> e : props.entrySet())
       result.put(e.getKey(), JacksonUtils.toObject(e.getValue()));
-    return result;
+    return Collections.unmodifiableMap(result);
   }
 
   void writeProps(JsonGenerator gen) throws IOException {
diff --git a/lang/java/avro/src/main/java/org/apache/avro/Protocol.java b/lang/java/avro/src/main/java/org/apache/avro/Protocol.java
index f381dd6..b54a51c 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/Protocol.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/Protocol.java
@@ -38,7 +38,6 @@ import org.apache.avro.Schema.Field;
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.node.TextNode;
 
 /** A set of messages forming an application protocol.
  * <p> A protocol consists of:
@@ -85,20 +84,22 @@ public class Protocol extends JsonProperties {
 
     /** Construct a message. */
     private Message(String name, String doc,
-                    Map<String,?> propMap, Schema request) {
+                    JsonProperties propMap, Schema request) {
       super(MESSAGE_RESERVED);
       this.name = name;
       this.doc = doc;
       this.request = request;
 
-      if (propMap != null)                        // copy props
-        for (Map.Entry<String,?> prop : propMap.entrySet()) {
-          Object value = prop.getValue();
-          this.addProp(prop.getKey(),
-                       value instanceof String
-                       ? TextNode.valueOf((String)value)
-                       : (JsonNode)value);
-        }
+      if (propMap != null)
+        // copy props
+        addAllProps(propMap);
+    }
+    private Message(String name, String doc,
+        Map<String, ?> propMap, Schema request) {
+      super(MESSAGE_RESERVED, propMap);
+      this.name = name;
+      this.doc = doc;
+      this.request = request;
     }
 
     /** The name of this message. */
@@ -118,7 +119,7 @@ public class Protocol extends JsonProperties {
     public String toString() {
       try {
         StringWriter writer = new StringWriter();
-        JsonGenerator gen = Schema.FACTORY.createJsonGenerator(writer);
+        JsonGenerator gen = Schema.FACTORY.createGenerator(writer);
         toJson(gen);
         gen.flush();
         return writer.toString();
@@ -170,6 +171,12 @@ public class Protocol extends JsonProperties {
       this.response = response;
       this.errors = errors;
     }
+    private TwoWayMessage(String name, String doc, JsonProperties propMap,
+        Schema request, Schema response, Schema errors) {
+      super(name, doc, propMap, request);
+      this.response = response;
+      this.errors = errors;
+    }
 
     @Override public Schema getResponse() { return response; }
     @Override public Schema getErrors() { return errors; }
@@ -278,36 +285,45 @@ public class Protocol extends JsonProperties {
   /** Create a one-way message. */
   @Deprecated
   public Message createMessage(String name, String doc, Schema request) {
-    return createMessage(name, doc, new LinkedHashMap<String,String>(),request);
+    return new Message(name, doc, new LinkedHashMap<String,String>(), request);
   }
 
   /** Create a one-way message using the {@code name}, {@code doc}, and {@code props} of {@code m}. */
   public Message createMessage(Message m, Schema request) {
-    return createMessage(m.getName(), m.getDoc(), m.getJsonProps(), request);
+    return new Message(name, doc, m, request);
   }
 
   /** Create a one-way message. */
   public <T> Message createMessage(String name, String doc,
-                                   Map<String,T> propMap, Schema request) {
+                                  JsonProperties propMap, Schema request) {
+    return new Message(name, doc, propMap, request);
+  }
+  /** Create a one-way message. */
+  public <T> Message createMessage(String name, String doc,
+                                   Map<String,?> propMap, Schema request) {
     return new Message(name, doc, propMap, request);
   }
 
   /** Create a two-way message. */
   @Deprecated
-  public Message createMessage(String name, String doc, Schema request,
-                               Schema response, Schema errors) {
-    return createMessage(name, doc, new LinkedHashMap<String,String>(),
-                         request, response, errors);
+  public Message createMessage(String name, String doc, Schema request, Schema response, Schema errors) {
+    return new TwoWayMessage(name, doc, new LinkedHashMap<String,String>(), request, response, errors);
   }
 
   /** Create a two-way message using the {@code name}, {@code doc}, and {@code props} of {@code m}. */
   public Message createMessage(Message m, Schema request, Schema response, Schema errors) {
-    return createMessage(m.getName(), m.getDoc(), m.getJsonProps(), request, response, errors);
+    return new TwoWayMessage(m.getName(), m.getDoc(), m, request, response, errors);
   }
 
   /** Create a two-way message. */
   public <T> Message createMessage(String name, String doc,
-                                   Map<String,T> propMap, Schema request,
+                                   JsonProperties propMap, Schema request,
+                                   Schema response, Schema errors) {
+    return new TwoWayMessage(name, doc, propMap, request, response, errors);
+  }
+  /** Create a two-way message. */
+  public <T> Message createMessage(String name, String doc,
+                                   Map<String,?> propMap, Schema request,
                                    Schema response, Schema errors) {
     return new TwoWayMessage(name, doc, propMap, request, response, errors);
   }
@@ -338,7 +354,7 @@ public class Protocol extends JsonProperties {
   public String toString(boolean pretty) {
     try {
       StringWriter writer = new StringWriter();
-      JsonGenerator gen = Schema.FACTORY.createJsonGenerator(writer);
+      JsonGenerator gen = Schema.FACTORY.createGenerator(writer);
       if (pretty) gen.useDefaultPrettyPrinter();
       toJson(gen);
       gen.flush();
@@ -386,12 +402,12 @@ public class Protocol extends JsonProperties {
 
   /** Read a protocol from a Json file. */
   public static Protocol parse(File file) throws IOException {
-    return parse(Schema.FACTORY.createJsonParser(file));
+    return parse(Schema.FACTORY.createParser(file));
   }
 
   /** Read a protocol from a Json stream. */
   public static Protocol parse(InputStream stream) throws IOException {
-    return parse(Schema.FACTORY.createJsonParser(stream));
+    return parse(Schema.FACTORY.createParser(stream));
   }
 
   /** Read a protocol from one or more json strings */
@@ -405,7 +421,7 @@ public class Protocol extends JsonProperties {
   /** Read a protocol from a Json string. */
   public static Protocol parse(String string) {
     try {
-      return parse(Schema.FACTORY.createJsonParser
+      return parse(Schema.FACTORY.createParser
                    (new ByteArrayInputStream(string.getBytes("UTF-8"))));
     } catch (IOException e) {
       throw new AvroRuntimeException(e);
diff --git a/lang/java/avro/src/main/java/org/apache/avro/Schema.java b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
index e08331e..e99143d 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/Schema.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
@@ -138,7 +138,7 @@ public abstract class Schema extends JsonProperties {
 
   int hashCode = NO_HASHCODE;
 
-  @Override public void addProp(String name, JsonNode value) {
+  @Override public void addProp(String name, String value) {
     super.addProp(name, value);
     hashCode = NO_HASHCODE;
   }
@@ -430,16 +430,13 @@ public abstract class Schema extends JsonProperties {
 
     Field(String name, Schema schema, String doc,
         JsonNode defaultValue) {
-      this(name, schema, doc, defaultValue, Order.ASCENDING);
+      this(name, schema, doc, defaultValue, true, Order.ASCENDING);
     }
-
-    /** @deprecated use {@link #Field(String, Schema, String, Object, Order)} */
-    @Deprecated
-    public Field(String name, Schema schema, String doc,
+    Field(String name, Schema schema, String doc,
         JsonNode defaultValue, Order order) {
       this(name, schema, doc, defaultValue, true, order);
     }
-    public Field(String name, Schema schema, String doc,
+    Field(String name, Schema schema, String doc,
                  JsonNode defaultValue, boolean validateDefault, Order order) {
       super(FIELD_RESERVED);
       this.name = validateName(name);
@@ -492,7 +489,7 @@ public abstract class Schema extends JsonProperties {
      */
     public Object defaultVal() { return JacksonUtils.toObject(defaultValue, schema); }
     public Order order() { return order; }
-    @Deprecated public Map<String,String> props() { return getProps(); }
+
     public void addAlias(String alias) {
       if (aliases == null)
         this.aliases = new LinkedHashSet<>();
@@ -1158,6 +1155,7 @@ public abstract class Schema extends JsonProperties {
   }
 
   static class Names extends LinkedHashMap<Name, Schema> {
+    private static final long serialVersionUID = 1L;
     private String space;                         // default namespace
 
     public Names() {}
diff --git a/lang/java/avro/src/test/java/org/apache/avro/TestSchemaBuilder.java b/lang/java/avro/src/test/java/org/apache/avro/TestSchemaBuilder.java
index fa6370c..7046fb3 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/TestSchemaBuilder.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/TestSchemaBuilder.java
@@ -84,8 +84,7 @@ public class TestSchemaBuilder {
       .prop("p2", "v2")
       .prop("p2", "v2real") // overwrite
       .endInt();
-    @SuppressWarnings("deprecation")
-    int size = s.getProps().size();
+    int size = s.getObjectProps().size();
     Assert.assertEquals(2, size);
     Assert.assertEquals("v1", s.getProp("p1"));
     Assert.assertEquals("v2real", s.getProp("p2"));
@@ -103,13 +102,6 @@ public class TestSchemaBuilder {
       .prop("stringProp", "abc" )
       .endInt();
 
-    //string properties
-    @SuppressWarnings("deprecation")
-    Map<String, String> stringProps = s.getProps();
-    Assert.assertEquals(2, stringProps.size());
-    Assert.assertEquals("ABC", stringProps.get("byteProp"));
-    Assert.assertEquals("abc", stringProps.get("stringProp"));
-
     //object properties
     Assert.assertEquals(7, s.getObjectProps().size());
     Assert.assertTrue(s.getObjectProp("booleanProp") instanceof Boolean);
@@ -148,12 +140,6 @@ public class TestSchemaBuilder {
 
     Schema.Field f = s.getField("myField");
 
-    //string properties
-    @SuppressWarnings("deprecation")
-    Map<String, String> stringProps = f.getProps();
-    Assert.assertEquals(2, stringProps.size());
-    Assert.assertEquals("ABC", stringProps.get("byteProp"));
-    Assert.assertEquals("abc", stringProps.get("stringProp"));
 
     //object properties
     Assert.assertEquals(7, f.getObjectProps().size());
@@ -191,11 +177,6 @@ public class TestSchemaBuilder {
       .prop("arrayProp", values)
       .endInt();
 
-    //string properties
-    @SuppressWarnings("deprecation")
-    int size = s.getProps().size();
-    Assert.assertEquals(0, size);
-
     //object properties
     Assert.assertEquals(1, s.getObjectProps().size());
 
@@ -235,11 +216,6 @@ public class TestSchemaBuilder {
 
     Schema.Field f = s.getField("myField");
 
-    //string properties
-    @SuppressWarnings("deprecation")
-    int size = f.getProps().size();
-    Assert.assertEquals(0, size);
-
     //object properties
     Assert.assertEquals(1, f.getObjectProps().size());
 
@@ -274,11 +250,6 @@ public class TestSchemaBuilder {
       .prop("mapProp", values)
       .endInt();
 
-    //string properties
-    @SuppressWarnings("deprecation")
-    int size = s.getProps().size();
-    Assert.assertEquals(0, size);
-    Assert.assertEquals(1, s.getObjectProps().size());
 
     //object properties
     Assert.assertTrue(s.getObjectProp("mapProp") instanceof Map);
@@ -324,12 +295,6 @@ public class TestSchemaBuilder {
 
     Schema.Field f = s.getField("myField");
 
-    //string properties
-    @SuppressWarnings("deprecation")
-    int size = f.getProps().size();
-    Assert.assertEquals(0, size);
-    Assert.assertEquals(1, f.getObjectProps().size());
-
     //object properties
     Assert.assertTrue(f.getObjectProp("mapProp") instanceof Map);
     @SuppressWarnings("unchecked")
diff --git a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/SchemaResolver.java b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/SchemaResolver.java
index 21ab3dd..2aa9885 100644
--- a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/SchemaResolver.java
+++ b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/SchemaResolver.java
@@ -107,13 +107,13 @@ final class SchemaResolver {
       if (value.isOneWay()) {
         Schema replacement = resolve(replacements, value.getRequest(), protocol);
         nvalue = result.createMessage(value.getName(), value.getDoc(),
-            value.getObjectProps(), replacement);
+            value, replacement);
       } else {
         Schema request = resolve(replacements, value.getRequest(), protocol);
         Schema response = resolve(replacements, value.getResponse(), protocol);
         Schema errors = resolve(replacements, value.getErrors(), protocol);
         nvalue = result.createMessage(value.getName(), value.getDoc(),
-            value.getObjectProps(), request, response, errors);
+            value, request, response, errors);
       }
       result.getMessages().put(entry.getKey(), nvalue);
     }
diff --git a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/IDLProtocolMojo.java b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/IDLProtocolMojo.java
index f82d942..92b7b40 100644
--- a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/IDLProtocolMojo.java
+++ b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/IDLProtocolMojo.java
@@ -65,7 +65,6 @@ public class IDLProtocolMojo extends AbstractAvroMojo {
     try {
       @SuppressWarnings("rawtypes")
       List runtimeClasspathElements = project.getRuntimeClasspathElements();
-      Idl parser;
 
       List<URL> runtimeUrls = new ArrayList<>();
 
@@ -83,19 +82,21 @@ public class IDLProtocolMojo extends AbstractAvroMojo {
 
       URLClassLoader projPathLoader = new URLClassLoader
           (runtimeUrls.toArray(new URL[0]), Thread.currentThread().getContextClassLoader());
-        parser = new Idl(new File(sourceDirectory, filename), projPathLoader);
-
-      Protocol p = parser.CompilationUnit();
-      String json = p.toString(true);
-      Protocol protocol = Protocol.parse(json);
-      SpecificCompiler compiler = new SpecificCompiler(protocol, getDateTimeLogicalTypeImplementation());
-      compiler.setStringType(GenericData.StringType.valueOf(stringType));
-      compiler.setTemplateDir(templateDirectory);
-      compiler.setFieldVisibility(getFieldVisibility());
-      compiler.setCreateSetters(createSetters);
-      compiler.setEnableDecimalLogicalType(enableDecimalLogicalType);
-      compiler.setOutputCharacterEncoding(project.getProperties().getProperty("project.build.sourceEncoding"));
-      compiler.compileToDestination(null, outputDirectory);
+
+      try (Idl parser = new Idl(new File(sourceDirectory, filename), projPathLoader)) {
+
+        Protocol p = parser.CompilationUnit();
+        String json = p.toString(true);
+        Protocol protocol = Protocol.parse(json);
+        SpecificCompiler compiler = new SpecificCompiler(protocol, getDateTimeLogicalTypeImplementation());
+        compiler.setStringType(GenericData.StringType.valueOf(stringType));
+        compiler.setTemplateDir(templateDirectory);
+        compiler.setFieldVisibility(getFieldVisibility());
+        compiler.setCreateSetters(createSetters);
+        compiler.setEnableDecimalLogicalType(enableDecimalLogicalType);
+        compiler.setOutputCharacterEncoding(project.getProperties().getProperty("project.build.sourceEncoding"));
+        compiler.compileToDestination(null, outputDirectory);
+      }
     } catch (ParseException e) {
       throw new IOException(e);
     } catch (DependencyResolutionRequiredException drre) {