Set the length fields of memcache messages automatically

Motivation:

People need to set all length fields manually when creating a memcache message and it's error prone. See #2736 for more dicussion.

Modifications:

This patch adds the logic to update the keyLength, extrasLength and totalBodyLength when key, extras or content is set.

Result:

The length fields of memcache messages will be updated automatically.

codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/AbstractBinaryMemcacheMessage.java

@@ -52,7 +52,10 @@ public abstract class AbstractBinaryMemcacheMessage
      */
     protected AbstractBinaryMemcacheMessage(ByteBuf key, ByteBuf extras) {
         this.key = key;
+        keyLength = key == null ? 0 : (short) key.readableBytes();
         this.extras = extras;
+        extrasLength = extras == null ? 0 : (byte) extras.readableBytes();
+        totalBodyLength = keyLength + extrasLength;
     }
 
     @Override
@@ -71,6 +74,9 @@ public abstract class AbstractBinaryMemcacheMessage
             this.key.release();
         }
         this.key = key;
+        short oldKeyLength = keyLength;
+        keyLength = key == null ? 0 : (short) key.readableBytes();
+        totalBodyLength  = totalBodyLength + keyLength - oldKeyLength;
         return this;
     }
 
@@ -80,6 +86,9 @@ public abstract class AbstractBinaryMemcacheMessage
             this.extras.release();
         }
         this.extras = extras;
+        short oldExtrasLength = extrasLength;
+        extrasLength = extras == null ? 0 : (byte) extras.readableBytes();
+        totalBodyLength = totalBodyLength + extrasLength - oldExtrasLength;
         return this;
     }
 
@@ -143,8 +152,14 @@ public abstract class AbstractBinaryMemcacheMessage
         return extrasLength;
     }
 
-    @Override
+    /**
-    public BinaryMemcacheMessage setExtrasLength(byte extrasLength) {
+     * Set the extras length of the message.
+     * <p/>
+     * This may be 0, since the extras content is optional.
+     *
+     * @param extrasLength the extras length.
+     */
+    BinaryMemcacheMessage setExtrasLength(byte extrasLength) {
         this.extrasLength = extrasLength;
         return this;
     }
@@ -154,8 +169,14 @@ public abstract class AbstractBinaryMemcacheMessage
         return keyLength;
     }
 
-    @Override
+    /**
-    public BinaryMemcacheMessage setKeyLength(short keyLength) {
+     * Set the key length of the message.
+     * <p/>
+     * This may be 0, since the key is optional.
+     *
+     * @param keyLength the key length to use.
+     */
+    BinaryMemcacheMessage setKeyLength(short keyLength) {
         this.keyLength = keyLength;
         return this;
     }

codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/BinaryMemcacheMessage.java

@@ -68,15 +68,6 @@ public interface BinaryMemcacheMessage extends MemcacheMessage {
      */
     short keyLength();
 
-    /**
-     * Set the key length of the message.
-     * <p/>
-     * This may be 0, since the key is optional.
-     *
-     * @param keyLength the key length to use.
-     */
-    BinaryMemcacheMessage setKeyLength(short keyLength);
-
     /**
      * Return the extras length of the message.
      * <p/>
@@ -86,15 +77,6 @@ public interface BinaryMemcacheMessage extends MemcacheMessage {
      */
     byte extrasLength();
 
-    /**
-     * Set the extras length of the message.
-     * <p/>
-     * This may be 0, since the extras content is optional.
-     *
-     * @param extrasLength the extras length.
-     */
-    BinaryMemcacheMessage setExtrasLength(byte extrasLength);
-
     /**
      * Returns the data type of the message.
      *

codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/BinaryMemcacheObjectAggregator.java

@@ -55,7 +55,7 @@ public class BinaryMemcacheObjectAggregator extends AbstractMemcacheObjectAggreg
     private static FullBinaryMemcacheRequest toFullRequest(BinaryMemcacheRequest request, ByteBuf content) {
         ByteBuf key = request.key() == null ? null : request.key().retain();
         ByteBuf extras = request.extras() == null ? null : request.extras().retain();
-        FullBinaryMemcacheRequest fullRequest =
+        DefaultFullBinaryMemcacheRequest fullRequest =
                 new DefaultFullBinaryMemcacheRequest(key, extras, content);
 
         fullRequest.setMagic(request.magic());
@@ -74,7 +74,7 @@ public class BinaryMemcacheObjectAggregator extends AbstractMemcacheObjectAggreg
     private static FullBinaryMemcacheResponse toFullResponse(BinaryMemcacheResponse response, ByteBuf content) {
         ByteBuf key = response.key() == null ? null : response.key().retain();
         ByteBuf extras = response.extras() == null ? null : response.extras().retain();
-        FullBinaryMemcacheResponse fullResponse =
+        DefaultFullBinaryMemcacheResponse fullResponse =
                 new DefaultFullBinaryMemcacheResponse(key, extras, content);
 
         fullResponse.setMagic(response.magic());

codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/BinaryMemcacheRequestDecoder.java

@@ -34,7 +34,7 @@ public class BinaryMemcacheRequestDecoder
 
     @Override
     protected BinaryMemcacheRequest decodeHeader(ByteBuf in) {
-        BinaryMemcacheRequest header = new DefaultBinaryMemcacheRequest();
+        DefaultBinaryMemcacheRequest header = new DefaultBinaryMemcacheRequest();
         header.setMagic(in.readByte());
         header.setOpcode(in.readByte());
         header.setKeyLength(in.readShort());

codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/BinaryMemcacheResponseDecoder.java

@@ -34,7 +34,7 @@ public class BinaryMemcacheResponseDecoder
 
     @Override
     protected BinaryMemcacheResponse decodeHeader(ByteBuf in) {
-        BinaryMemcacheResponse header = new DefaultBinaryMemcacheResponse();
+        DefaultBinaryMemcacheResponse header = new DefaultBinaryMemcacheResponse();
         header.setMagic(in.readByte());
         header.setOpcode(in.readByte());
         header.setKeyLength(in.readShort());

codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/DefaultFullBinaryMemcacheRequest.java

@@ -51,6 +51,7 @@ public class DefaultFullBinaryMemcacheRequest extends DefaultBinaryMemcacheReque
         }
 
         this.content = content;
+        setTotalBodyLength(keyLength() + extrasLength() + content.readableBytes());
     }
 
     @Override

codec-memcache/src/main/java/io/netty/handler/codec/memcache/binary/DefaultFullBinaryMemcacheResponse.java

@@ -51,6 +51,7 @@ public class DefaultFullBinaryMemcacheResponse extends DefaultBinaryMemcacheResp
         }
 
         this.content = content;
+        setTotalBodyLength(keyLength() + extrasLength() + content.readableBytes());
     }
 
     @Override

codec-memcache/src/test/java/io/netty/handler/codec/memcache/binary/BinaryMemcacheDecoderTest.java

@@ -259,8 +259,6 @@ public class BinaryMemcacheDecoderTest {
         ByteBuf key = Unpooled.copiedBuffer("Netty", CharsetUtil.UTF_8);
         ByteBuf extras = Unpooled.copiedBuffer("extras", CharsetUtil.UTF_8);
         BinaryMemcacheRequest request = new DefaultBinaryMemcacheRequest(key, extras);
-        request.setKeyLength((short) key.readableBytes());
-        request.setExtrasLength((byte) extras.readableBytes());
 
         assertTrue(channel.writeOutbound(request));
         assertTrue(channel.writeInbound(channel.outboundMessages().toArray()));

codec-memcache/src/test/java/io/netty/handler/codec/memcache/binary/BinaryMemcacheEncoderTest.java

@@ -86,7 +86,6 @@ public class BinaryMemcacheEncoderTest {
         int extrasLength = extras.readableBytes();
 
         BinaryMemcacheRequest request = new DefaultBinaryMemcacheRequest(Unpooled.EMPTY_BUFFER, extras);
-        request.setExtrasLength((byte) extrasLength);
 
         boolean result = channel.writeOutbound(request);
         assertThat(result, is(true));
@@ -104,7 +103,6 @@ public class BinaryMemcacheEncoderTest {
         int keyLength = key.readableBytes();
 
         BinaryMemcacheRequest request = new DefaultBinaryMemcacheRequest(key);
-        request.setKeyLength((byte) keyLength);
 
         boolean result = channel.writeOutbound(request);
         assertThat(result, is(true));

codec-memcache/src/test/java/io/netty/handler/codec/memcache/binary/BinaryMemcacheMessageTest.java

@@ -0,0 +1,121 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project 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 io.netty.handler.codec.memcache.binary;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import io.netty.util.CharsetUtil;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class BinaryMemcacheMessageTest {
+
+    @Test
+    public void testSetLengths() {
+        ByteBuf key = Unpooled.copiedBuffer("Netty  Rocks!", CharsetUtil.UTF_8);
+        ByteBuf extras = Unpooled.copiedBuffer("some extras", CharsetUtil.UTF_8);
+        ByteBuf content = Unpooled.copiedBuffer("content", CharsetUtil.UTF_8);
+        try {
+            testSettingLengths(new DefaultBinaryMemcacheRequest(), 0, 0, 0);
+            testSettingLengths(new DefaultBinaryMemcacheRequest(key.retain()), key.readableBytes(), 0, 0);
+            testSettingLengths(new DefaultBinaryMemcacheRequest(key.retain(), extras.retain()),
+                    key.readableBytes(), extras.readableBytes(), 0);
+
+            testSettingLengths(new DefaultBinaryMemcacheResponse(), 0, 0, 0);
+            testSettingLengths(new DefaultBinaryMemcacheResponse(key.retain()), key.readableBytes(), 0, 0);
+            testSettingLengths(new DefaultBinaryMemcacheResponse(key.retain(), extras.retain()),
+                    key.readableBytes(), extras.readableBytes(), 0);
+
+            testSettingLengths(new DefaultFullBinaryMemcacheRequest(key.retain(), extras.retain()),
+                    key.readableBytes(), extras.readableBytes(), 0);
+            testSettingLengths(new DefaultFullBinaryMemcacheRequest(null, extras.retain()),
+                    0, extras.readableBytes(), 0);
+            testSettingLengths(new DefaultFullBinaryMemcacheRequest(key.retain(), null),
+                    key.readableBytes(), 0, 0);
+            testSettingLengths(new DefaultFullBinaryMemcacheRequest(null, null), 0, 0, 0);
+            testSettingLengths(new DefaultFullBinaryMemcacheRequest(key.retain(), extras.retain(), content.retain()),
+                    key.readableBytes(), extras.readableBytes(), content.readableBytes());
+            testSettingLengths(new DefaultFullBinaryMemcacheRequest(null, extras.retain(), content.retain()),
+                    0, extras.readableBytes(), content.readableBytes());
+            testSettingLengths(new DefaultFullBinaryMemcacheRequest(key.retain(), null, content.retain()),
+                    key.readableBytes(), 0, content.readableBytes());
+            testSettingLengths(new DefaultFullBinaryMemcacheRequest(null, null, content.retain()),
+                    0, 0, content.readableBytes());
+
+            testSettingLengths(new DefaultFullBinaryMemcacheResponse(key.retain(), extras.retain()),
+                    key.readableBytes(), extras.readableBytes(), 0);
+            testSettingLengths(new DefaultFullBinaryMemcacheResponse(null, extras.retain()),
+                    0, extras.readableBytes(), 0);
+            testSettingLengths(new DefaultFullBinaryMemcacheResponse(key.retain(), null),
+                    key.readableBytes(), 0, 0);
+            testSettingLengths(new DefaultFullBinaryMemcacheResponse(null, null), 0, 0, 0);
+            testSettingLengths(new DefaultFullBinaryMemcacheResponse(key.retain(), extras.retain(), content.retain()),
+                    key.readableBytes(), extras.readableBytes(), content.readableBytes());
+            testSettingLengths(new DefaultFullBinaryMemcacheResponse(null, extras.retain(), content.retain()),
+                    0, extras.readableBytes(), content.readableBytes());
+            testSettingLengths(new DefaultFullBinaryMemcacheResponse(key.retain(), null, content.retain()),
+                    key.readableBytes(), 0, content.readableBytes());
+            testSettingLengths(new DefaultFullBinaryMemcacheResponse(null, null, content.retain()),
+                    0, 0, content.readableBytes());
+        } finally {
+            key.release();
+            extras.release();
+            content.release();
+        }
+    }
+
+    private static void testSettingLengths(BinaryMemcacheMessage message,
+                                    int initialKeyLength, int initialExtrasLength, int contentLength) {
+        ByteBuf key = Unpooled.copiedBuffer("netty", CharsetUtil.UTF_8);
+        ByteBuf extras = Unpooled.copiedBuffer("extras", CharsetUtil.UTF_8);
+        ByteBuf key2 = Unpooled.copiedBuffer("netty!", CharsetUtil.UTF_8);
+        ByteBuf extras2 = Unpooled.copiedBuffer("extras!", CharsetUtil.UTF_8);
+        try {
+            assertEquals(initialKeyLength, message.keyLength());
+            assertEquals(initialExtrasLength, message.extrasLength());
+            assertEquals(initialKeyLength + initialExtrasLength + contentLength, message.totalBodyLength());
+
+            message.setKey(key.retain());
+            assertEquals(key.readableBytes(), message.keyLength());
+            assertEquals(initialExtrasLength, message.extrasLength());
+            assertEquals(key.readableBytes() + initialExtrasLength + contentLength, message.totalBodyLength());
+
+            message.setExtras(extras.retain());
+            assertEquals(key.readableBytes(), message.keyLength());
+            assertEquals(extras.readableBytes(), message.extrasLength());
+            assertEquals(key.readableBytes() + extras.readableBytes() + contentLength, message.totalBodyLength());
+
+            // Replace the previous key
+            message.setKey(key2.retain());
+            assertEquals(key2.readableBytes(), message.keyLength());
+            assertEquals(extras.readableBytes(), message.extrasLength());
+            assertEquals(key2.readableBytes() + extras.readableBytes() + contentLength, message.totalBodyLength());
+
+            // Replace the previous extras
+            message.setExtras(extras2.retain());
+            assertEquals(key2.readableBytes(), message.keyLength());
+            assertEquals(extras2.readableBytes(), message.extrasLength());
+            assertEquals(key2.readableBytes() + extras2.readableBytes() + contentLength, message.totalBodyLength());
+        } finally {
+            key.release();
+            extras.release();
+            key2.release();
+            extras2.release();
+            message.release();
+        }
+    }
+}

codec-memcache/src/test/java/io/netty/handler/codec/memcache/binary/BinaryMemcacheObjectAggregatorTest.java

@@ -89,8 +89,6 @@ public class BinaryMemcacheObjectAggregatorTest {
         ByteBuf key = Unpooled.copiedBuffer("Netty", CharsetUtil.UTF_8);
         ByteBuf extras = Unpooled.copiedBuffer("extras", CharsetUtil.UTF_8);
         BinaryMemcacheRequest request = new DefaultBinaryMemcacheRequest(key, extras);
-        request.setKeyLength((short) key.readableBytes());
-        request.setExtrasLength((byte) extras.readableBytes());
 
         DefaultMemcacheContent content1 =
                 new DefaultMemcacheContent(Unpooled.copiedBuffer("Netty", CharsetUtil.UTF_8));

example/src/main/java/io/netty/example/memcache/binary/MemcacheClientHandler.java

@@ -41,8 +41,6 @@ public class MemcacheClientHandler extends ChannelDuplexHandler {
 
             BinaryMemcacheRequest req = new DefaultBinaryMemcacheRequest(key);
             req.setOpcode(BinaryMemcacheOpcodes.GET);
-            req.setKeyLength((short) key.readableBytes());
-            req.setTotalBodyLength(key.readableBytes());
 
             ctx.write(req, promise);
         } else if (command.startsWith("set ")) {
@@ -60,9 +58,6 @@ public class MemcacheClientHandler extends ChannelDuplexHandler {
 
             BinaryMemcacheRequest req = new DefaultFullBinaryMemcacheRequest(key, extras, content);
             req.setOpcode(BinaryMemcacheOpcodes.SET);
-            req.setKeyLength((short) key.readableBytes());
-            req.setExtrasLength((byte) 8);
-            req.setTotalBodyLength(key.readableBytes() + 8 + value.length());
 
             ctx.write(req, promise);
         } else {