From 592aaa1f060908ed9c8a32d387e0ad8f1fc030b8 Mon Sep 17 00:00:00 2001 From: Todd Lipcon Date: Fri, 9 Sep 2011 21:18:06 +0000 Subject: [PATCH] HADOOP-7328. When a serializer class is missing, return null, not throw an NPE. Contributed by Harsh J Chouraria. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1167363 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 ++ .../io/serializer/SerializationFactory.java | 29 +++++++----- .../serializer/TestSerializationFactory.java | 44 +++++++++++++++++++ 3 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/serializer/TestSerializationFactory.java diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 0812c05fe1..23ac34d306 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -573,6 +573,9 @@ Release 0.23.0 - Unreleased HADOOP-7598. Fix smart-apply-patch.sh to handle patching from a sub directory correctly. (Robert Evans via acmurthy) + HADOOP-7328. When a serializer class is missing, return null, not throw + an NPE. (Harsh J Chouraria via todd) + Release 0.22.0 - Unreleased INCOMPATIBLE CHANGES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/serializer/SerializationFactory.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/serializer/SerializationFactory.java index a48d114b18..52a0a253bb 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/serializer/SerializationFactory.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/serializer/SerializationFactory.java @@ -27,10 +27,10 @@ import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configured; +import org.apache.hadoop.fs.CommonConfigurationKeys; import org.apache.hadoop.io.serializer.avro.AvroReflectSerialization; import org.apache.hadoop.io.serializer.avro.AvroSpecificSerialization; import org.apache.hadoop.util.ReflectionUtils; -import org.apache.hadoop.util.StringUtils; /** *

@@ -50,14 +50,15 @@ public class SerializationFactory extends Configured { *

* Serializations are found by reading the io.serializations * property from conf, which is a comma-delimited list of - * classnames. + * classnames. *

*/ public SerializationFactory(Configuration conf) { super(conf); - for (String serializerName : conf.getStrings("io.serializations", - new String[]{WritableSerialization.class.getName(), - AvroSpecificSerialization.class.getName(), + for (String serializerName : conf.getStrings( + CommonConfigurationKeys.IO_SERIALIZATIONS_KEY, + new String[]{WritableSerialization.class.getName(), + AvroSpecificSerialization.class.getName(), AvroReflectSerialization.class.getName()})) { add(conf, serializerName); } @@ -67,27 +68,35 @@ public SerializationFactory(Configuration conf) { private void add(Configuration conf, String serializationName) { try { Class serializionClass = - (Class) conf.getClassByName(serializationName); + (Class) conf.getClassByName(serializationName); serializations.add((Serialization) - ReflectionUtils.newInstance(serializionClass, getConf())); + ReflectionUtils.newInstance(serializionClass, getConf())); } catch (ClassNotFoundException e) { LOG.warn("Serialization class not found: ", e); } } public Serializer getSerializer(Class c) { - return getSerialization(c).getSerializer(c); + Serialization serializer = getSerialization(c); + if (serializer != null) { + return serializer.getSerializer(c); + } + return null; } public Deserializer getDeserializer(Class c) { - return getSerialization(c).getDeserializer(c); + Serialization serializer = getSerialization(c); + if (serializer != null) { + return serializer.getDeserializer(c); + } + return null; } @SuppressWarnings("unchecked") public Serialization getSerialization(Class c) { for (Serialization serialization : serializations) { if (serialization.accept(c)) { - return (Serialization) serialization; + return (Serialization) serialization; } } return null; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/serializer/TestSerializationFactory.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/serializer/TestSerializationFactory.java new file mode 100644 index 0000000000..18c2637ec5 --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/serializer/TestSerializationFactory.java @@ -0,0 +1,44 @@ +/** + * 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.hadoop.io.serializer; + +import org.junit.Test; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertNotNull; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.io.Writable; + +public class TestSerializationFactory { + + @Test + public void testSerializerAvailability() { + Configuration conf = new Configuration(); + SerializationFactory factory = new SerializationFactory(conf); + // Test that a valid serializer class is returned when its present + assertNotNull("A valid class must be returned for default Writable Serde", + factory.getSerializer(Writable.class)); + assertNotNull("A valid class must be returned for default Writable serDe", + factory.getDeserializer(Writable.class)); + // Test that a null is returned when none can be found. + assertNull("A null should be returned if there are no serializers found.", + factory.getSerializer(TestSerializationFactory.class)); + assertNull("A null should be returned if there are no deserializers found", + factory.getDeserializer(TestSerializationFactory.class)); + } +}