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 <[email protected]> Authored: Mon Jul 25 14:32:46 2016 -0700 Committer: Ryan Blue <[email protected]> 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>
