You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by bl...@apache.org on 2016/10/30 21:09:57 UTC

avro git commit: AVRO-1882: ConcurrentHashMap with non-string keys fails in Java 1.8

Repository: avro
Updated Branches:
  refs/heads/master 8ecb2f130 -> 420824c13


AVRO-1882: ConcurrentHashMap with non-string keys fails in Java 1.8


Project: http://git-wip-us.apache.org/repos/asf/avro/repo
Commit: http://git-wip-us.apache.org/repos/asf/avro/commit/420824c1
Tree: http://git-wip-us.apache.org/repos/asf/avro/tree/420824c1
Diff: http://git-wip-us.apache.org/repos/asf/avro/diff/420824c1

Branch: refs/heads/master
Commit: 420824c13381c4014d102c8e51e231c694ddacf2
Parents: 8ecb2f1
Author: Sachin Goyal <sg...@L-SB8M1HEG8W-M.local>
Authored: Mon Jul 25 14:32:46 2016 -0700
Committer: Ryan Blue <bl...@apache.org>
Committed: Sun Oct 30 14:09:50 2016 -0700

----------------------------------------------------------------------
 CHANGES.txt                                     |  3 +
 .../java/org/apache/avro/reflect/MapEntry.java  | 60 ++++++++++++++++++++
 .../apache/avro/reflect/ReflectDatumWriter.java | 19 +++++--
 .../avro/reflect/TestNonStringMapKeys.java      | 42 +++++++++++---
 lang/java/pom.xml                               |  2 +-
 5 files changed, 112 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/avro/blob/420824c1/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 5d231f9..ac2eff9 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -84,6 +84,9 @@ Trunk (not yet released)
     AVRO-1929: Java: SchemaCompatibility fails to recognize
     string<->bytes promotion.  (Anders Sundelin via cutting)
 
+    AVRO-1882: Java: Fix ConcurrentHashMap with non-string keys.
+    (Sachin Goyal via blue)
+
 Avro 1.8.1 (14 May 2016)
 
   INCOMPATIBLE CHANGES

http://git-wip-us.apache.org/repos/asf/avro/blob/420824c1/lang/java/avro/src/main/java/org/apache/avro/reflect/MapEntry.java
----------------------------------------------------------------------
diff --git a/lang/java/avro/src/main/java/org/apache/avro/reflect/MapEntry.java b/lang/java/avro/src/main/java/org/apache/avro/reflect/MapEntry.java
new file mode 100644
index 0000000..701dacb
--- /dev/null
+++ b/lang/java/avro/src/main/java/org/apache/avro/reflect/MapEntry.java
@@ -0,0 +1,60 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.reflect;
+
+import java.util.Map;
+
+/**
+ * Class to make Avro immune from the naming variations of key/value fields
+ * among several {@link Map.Entry} implementations. If objects of this class
+ * are used instead of the regular ones obtained by {@link Map#entrySet()},
+ * then we need not worry about the actual field-names or any changes to them
+ * in the future.<BR>
+ * Example: {@code ConcurrentHashMap.MapEntry} does not name the fields as key/
+ * value in Java 1.8 while it used to do so in Java 1.7
+ *
+ * @param <K> Key of the map-entry
+ * @param <V> Value of the map-entry
+ */
+public class MapEntry<K, V> implements Map.Entry<K, V> {
+
+  K key;
+  V value;
+
+  public MapEntry(K key, V value) {
+    this.key = key;
+    this.value = value;
+  }
+
+  @Override
+  public K getKey() {
+    return key;
+  }
+
+  @Override
+  public V getValue() {
+    return value;
+  }
+
+  @Override
+  public V setValue(V value) {
+    V oldValue = this.value;
+    this.value = value;
+    return oldValue;
+  }
+}

http://git-wip-us.apache.org/repos/asf/avro/blob/420824c1/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java
----------------------------------------------------------------------
diff --git a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java
index fbdb9a5..682fd02 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java
@@ -18,8 +18,11 @@
 package org.apache.avro.reflect;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.avro.AvroRuntimeException;
 import org.apache.avro.Schema;
@@ -139,13 +142,19 @@ public class ReflectDatumWriter<T> extends SpecificDatumWriter<T> {
     else if (datum instanceof Short)
       datum = ((Short)datum).intValue();
     else if (datum instanceof Character)
-        datum = (int)(char)(Character)datum;
+      datum = (int)(char)(Character)datum;
     else if (datum instanceof Map && ReflectData.isNonStringMapSchema(schema)) {
-        // Maps with non-string keys are written as arrays.
-        // Schema for such maps is already changed. Here we
-        // just switch the map to a similar form too.
-        datum = ((Map)datum).entrySet();
+      // Maps with non-string keys are written as arrays.
+      // Schema for such maps is already changed. Here we
+      // just switch the map to a similar form too.
+      Set entries = ((Map)datum).entrySet();
+      List<Map.Entry> entryList = new ArrayList<Map.Entry>(entries.size());
+      for (Object obj: ((Map)datum).entrySet()) {
+          Map.Entry e = (Map.Entry)obj;
+          entryList.add(new MapEntry(e.getKey(), e.getValue()));
       }
+      datum = entryList;
+    }
     try {
       super.write(schema, datum, out);
     } catch (NullPointerException e) {            // improve error message

http://git-wip-us.apache.org/repos/asf/avro/blob/420824c1/lang/java/avro/src/test/java/org/apache/avro/reflect/TestNonStringMapKeys.java
----------------------------------------------------------------------
diff --git a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestNonStringMapKeys.java b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestNonStringMapKeys.java
index 3267529..26c7bba 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestNonStringMapKeys.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestNonStringMapKeys.java
@@ -22,11 +22,15 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map.Entry;
 
 import static org.junit.Assert.*;
 
+import java.util.TreeMap;
+import java.util.concurrent.ConcurrentHashMap;
+
 import org.apache.avro.Schema;
 import org.apache.avro.file.DataFileReader;
 import org.apache.avro.file.DataFileWriter;
@@ -34,10 +38,6 @@ import org.apache.avro.file.SeekableByteArrayInput;
 import org.apache.avro.generic.GenericArray;
 import org.apache.avro.generic.GenericDatumReader;
 import org.apache.avro.generic.GenericRecord;
-import org.apache.avro.reflect.ReflectData;
-import org.apache.avro.reflect.ReflectDatumReader;
-import org.apache.avro.reflect.ReflectDatumWriter;
-import org.apache.avro.reflect.ReflectData;
 import org.apache.avro.io.Decoder;
 import org.apache.avro.io.DecoderFactory;
 import org.apache.avro.io.Encoder;
@@ -206,16 +206,24 @@ public class TestNonStringMapKeys {
       assertEquals ("Foo", value.toString());
     }
     assertEquals (entity.getMap1(), entity.getMap2());
+    assertEquals (entity.getMap1(), entity.getMap3());
+    assertEquals (entity.getMap1(), entity.getMap4());
 
 
     ReflectData rdata = ReflectData.get();
     Schema schema = rdata.getSchema(SameMapSignature.class);
     Schema map1schema = schema.getField("map1").schema().getElementType();
     Schema map2schema = schema.getField("map2").schema().getElementType();
+    Schema map3schema = schema.getField("map3").schema().getElementType();
+    Schema map4schema = schema.getField("map4").schema().getElementType();
     log ("Schema for map1 = " + map1schema);
     log ("Schema for map2 = " + map2schema);
+    log ("Schema for map3 = " + map3schema);
+    log ("Schema for map4 = " + map4schema);
     assertEquals (map1schema.getFullName(), "org.apache.avro.reflect.PairIntegerString");
     assertEquals (map1schema, map2schema);
+    assertEquals (map1schema, map3schema);
+    assertEquals (map1schema, map4schema);
 
 
     byte[] jsonBytes = testJsonEncoder (testType, entityObj1);
@@ -365,8 +373,12 @@ public class TestNonStringMapKeys {
     SameMapSignature obj = new SameMapSignature();
     obj.setMap1(new HashMap<Integer, String>());
     obj.getMap1().put(1, "Foo");
-    obj.setMap2(new HashMap<Integer, String>());
+    obj.setMap2(new ConcurrentHashMap<Integer, String>());
     obj.getMap2().put(1, "Foo");
+    obj.setMap3(new LinkedHashMap<Integer, String>());
+    obj.getMap3().put(1, "Foo");
+    obj.setMap4(new TreeMap<Integer, String>());
+    obj.getMap4().put(1, "Foo");
     return obj;
   }
 
@@ -492,7 +504,9 @@ class EmployeeInfo2 {
 class SameMapSignature {
 
   HashMap<Integer, String> map1;
-  HashMap<Integer, String> map2;
+  ConcurrentHashMap<Integer, String> map2;
+  LinkedHashMap<Integer, String> map3;
+  TreeMap<Integer, String> map4;
 
   public HashMap<Integer, String> getMap1() {
     return map1;
@@ -500,10 +514,22 @@ class SameMapSignature {
   public void setMap1(HashMap<Integer, String> map1) {
     this.map1 = map1;
   }
-  public HashMap<Integer, String> getMap2() {
+  public ConcurrentHashMap<Integer, String> getMap2() {
     return map2;
   }
-  public void setMap2(HashMap<Integer, String> map2) {
+  public void setMap2(ConcurrentHashMap<Integer, String> map2) {
     this.map2 = map2;
   }
+  public LinkedHashMap<Integer, String> getMap3() {
+    return map3;
+  }
+  public void setMap3(LinkedHashMap<Integer, String> map3) {
+    this.map3 = map3;
+  }
+  public TreeMap<Integer, String> getMap4() {
+    return map4;
+  }
+  public void setMap4(TreeMap<Integer, String> map4) {
+    this.map4 = map4;
+  }
 }

http://git-wip-us.apache.org/repos/asf/avro/blob/420824c1/lang/java/pom.xml
----------------------------------------------------------------------
diff --git a/lang/java/pom.xml b/lang/java/pom.xml
index a232df7..8afcefd 100644
--- a/lang/java/pom.xml
+++ b/lang/java/pom.xml
@@ -86,7 +86,7 @@
     <source-plugin.version>2.3</source-plugin.version>
     <surefire-plugin.version>2.17</surefire-plugin.version>
     <file-management.version>1.2.1</file-management.version>
-    <shade-plugin.version>2.4.3</shade-plugin.version>
+    <shade-plugin.version>1.7.1</shade-plugin.version>
     <archetype-plugin.version>2.2</archetype-plugin.version>
   </properties>