ZOOKEEPER-2517: jute.maxbuffer is ignored

ZOOKEEPER-2517:jute.maxbuffer is ignored

Author: arshadmohammad <arshad.mohammad.k@gmail.com>

Reviewers: Michael Han <hanm@apache.org>, Rakesh Radhakrishnan <rakeshr@apache.org>

Closes #113 from arshadmohammad/ZOOKEEPER-2517

src/java/main/org/apache/zookeeper/ClientCnxnSocket.java

@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.nio.ByteBuffer;
+import java.text.MessageFormat;
 import java.util.List;
 import java.util.concurrent.LinkedBlockingDeque;
 
@@ -228,14 +229,20 @@ abstract class ClientCnxnSocket {
      */
     abstract void sendPacket(Packet p) throws IOException;
 
-    protected void initProperties() {
-        packetLen = Integer.getInteger(
-                clientConfig.getProperty(ZKConfig.JUTE_MAXBUFFER),
-                ZKClientConfig.CLIENT_MAX_PACKET_LENGTH_DEFAULT);
-        if (LOG.isDebugEnabled()) {
-            LOG.debug("{} is {}", ZKConfig.JUTE_MAXBUFFER, packetLen);
+    protected void initProperties() throws IOException {
+        try {
+            packetLen = clientConfig.getInt(ZKConfig.JUTE_MAXBUFFER,
+                    ZKClientConfig.CLIENT_MAX_PACKET_LENGTH_DEFAULT);
+            LOG.info("{} value is {} Bytes", ZKConfig.JUTE_MAXBUFFER,
+                    packetLen);
+        } catch (NumberFormatException e) {
+            String msg = MessageFormat.format(
+                    "Configured value {0} for property {1} can not be parsed to int",
+                    clientConfig.getProperty(ZKConfig.JUTE_MAXBUFFER),
+                    ZKConfig.JUTE_MAXBUFFER);
+            LOG.error(msg);
+            throw new IOException(msg);
         }
     }
 
-
 }

src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java

@@ -78,7 +78,7 @@ public class ClientCnxnSocketNetty extends ClientCnxnSocket {
     AtomicBoolean needSasl = new AtomicBoolean();
     Semaphore waitSasl = new Semaphore(0);
 
-    ClientCnxnSocketNetty(ZKClientConfig clientConfig) {
+    ClientCnxnSocketNetty(ZKClientConfig clientConfig) throws IOException {
         this.clientConfig = clientConfig;
         initProperties();
     }

src/java/main/org/apache/zookeeper/common/ZKConfig.java

@@ -223,4 +223,25 @@ public class ZKConfig {
         return Boolean.parseBoolean(getProperty(key));
     }
 
+    /**
+     * Get the value of the <code>key</code> property as an <code>int</code>. If
+     * property is not set, the provided <code>defaultValue</code> is returned
+     * 
+     * @param key
+     *            property key.
+     * @param defaultValue
+     *            default value.
+     * @throws NumberFormatException
+     *             when the value is invalid
+     * @return return property value as an <code>int</code>, or
+     *         <code>defaultValue</code>
+     */
+    public int getInt(String key, int defaultValue) {
+        String value = getProperty(key);
+        if (value != null) {
+            return Integer.parseInt(value.trim());
+        }
+        return defaultValue;
+    }
+
 }

src/java/test/org/apache/zookeeper/ClientCnxnSocketTest.java

@@ -0,0 +1,53 @@
+/**
+ * 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.zookeeper;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+
+import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.ZKConfig;
+import org.junit.Test;
+
+public class ClientCnxnSocketTest {
+
+    @Test
+    public void testWhenInvalidJuteMaxBufferIsConfiguredIOExceptionIsThrown() {
+        ZKClientConfig clientConfig = new ZKClientConfig();
+        String value = "SomeInvalidInt";
+        clientConfig.setProperty(ZKConfig.JUTE_MAXBUFFER, value);
+        // verify ClientCnxnSocketNIO creation
+        try {
+            new ClientCnxnSocketNIO(clientConfig);
+            fail("IOException is expected.");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().contains(value));
+        }
+        // verify ClientCnxnSocketNetty creation
+        try {
+            new ClientCnxnSocketNetty(clientConfig);
+            fail("IOException is expected.");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().contains(value));
+        }
+
+    }
+
+}

src/java/test/org/apache/zookeeper/client/ZKClientConfigTest.java

@@ -28,6 +28,7 @@ import static org.apache.zookeeper.client.ZKClientConfig.ZOOKEEPER_SERVER_REALM;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.io.FileOutputStream;
@@ -38,6 +39,7 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.zookeeper.common.ZKConfig;
 import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.junit.BeforeClass;
 import org.junit.Rule;
@@ -152,4 +154,38 @@ public class ZKClientConfigTest {
         assertTrue(conf.getProperty(ENABLE_CLIENT_SASL_KEY) != defaultValue);
     }
 
+    @Test
+    public void testIntegerRetrievalFromProperty() {
+        ZKClientConfig conf = new ZKClientConfig();
+        String prop = "UnSetProperty" + System.currentTimeMillis();
+        int defaultValue = 100;
+        // property is not set we should get the default value
+        int result = conf.getInt(prop, defaultValue);
+        assertEquals(defaultValue, result);
+
+        // property is set but can not be parsed to int, we should get the
+        // NumberFormatException
+        conf.setProperty(ZKConfig.JUTE_MAXBUFFER, "InvlaidIntValue123");
+        try {
+            result = conf.getInt(ZKConfig.JUTE_MAXBUFFER, defaultValue);
+            fail("NumberFormatException is expected");
+        } catch (NumberFormatException exception) {
+            // do nothing
+        }
+        assertEquals(defaultValue, result);
+
+        // property is set to an valid int, we should get the set value
+        int value = ZKClientConfig.CLIENT_MAX_PACKET_LENGTH_DEFAULT;
+        conf.setProperty(ZKConfig.JUTE_MAXBUFFER, Integer.toString(value));
+        result = conf.getInt(ZKConfig.JUTE_MAXBUFFER, defaultValue);
+        assertEquals(value, result);
+
+        // property is set but with white spaces
+        value = 12345;
+        conf.setProperty(ZKConfig.JUTE_MAXBUFFER,
+                " " + Integer.toString(value) + " ");
+        result = conf.getInt(ZKConfig.JUTE_MAXBUFFER, defaultValue);
+        assertEquals(value, result);
+    }
+
 }