codec-memcache: copy metadata in binary full request response (#9160)

Motivations
-----------
Calling `copy()`, `duplicate()` or `replace()` on `FullBinaryMemcacheResponse`
or `FullBinaryMemcacheRequest` instances should copy status, opCode, etc.
that are defined in `AbstractBinaryMemcacheMessage`.

Modifications
-------------
 - Modified duplicate, copy and replace methods in
DefaultFullBinaryMemcacheRequest and DefaultFullBinaryMemcacheResponse
to always copy metadata from parent classes.
 - Unit tests verifying duplicate, copy and replace methods for
DefaultFullBinaryMemcacheRequest and DefaultFullBinaryMemcacheResponse
copy buffers and metadata as expected.

Result
------
Calling copy(), duplicate() or replace() methods on
DefaultFullBinaryMemcacheRequest or DefaultFullBinaryMemcacheResponse
produces valid copies with all expected metadata.

Fixes #9159

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

@@ -78,7 +78,7 @@ public abstract class AbstractBinaryMemcacheMessage
         this.key = key;
         short oldKeyLength = keyLength;
         keyLength = key == null ? 0 : (short) key.readableBytes();
-        totalBodyLength  = totalBodyLength + keyLength - oldKeyLength;
+        totalBodyLength = totalBodyLength + keyLength - oldKeyLength;
         return this;
     }
 
@@ -232,4 +232,20 @@ public abstract class AbstractBinaryMemcacheMessage
         }
         return this;
     }
+
+    /**
+     * Copies special metadata hold by this instance to the provided instance
+     *
+     * @param dst The instance where to copy the metadata of this instance to
+     */
+    void copyMeta(AbstractBinaryMemcacheMessage dst) {
+        dst.magic = magic;
+        dst.opcode = opcode;
+        dst.keyLength = keyLength;
+        dst.extrasLength = extrasLength;
+        dst.dataType = dataType;
+        dst.totalBodyLength = totalBodyLength;
+        dst.opaque = opaque;
+        dst.cas = cas;
+    }
 }

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

@@ -92,4 +92,14 @@ public class DefaultBinaryMemcacheRequest extends AbstractBinaryMemcacheMessage
         super.touch(hint);
         return this;
     }
+
+    /**
+     * Copies special metadata hold by this instance to the provided instance
+     *
+     * @param dst The instance where to copy the metadata of this instance to
+     */
+    void copyMeta(DefaultBinaryMemcacheRequest dst) {
+        super.copyMeta(dst);
+        dst.reserved = reserved;
+    }
 }

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

@@ -41,7 +41,7 @@ public class DefaultBinaryMemcacheResponse extends AbstractBinaryMemcacheMessage
     /**
      * Create a new {@link DefaultBinaryMemcacheResponse} with the header and key.
      *
-     * @param key    the key to use
+     * @param key    the key to use.
      */
     public DefaultBinaryMemcacheResponse(ByteBuf key) {
         this(key, null);
@@ -92,4 +92,14 @@ public class DefaultBinaryMemcacheResponse extends AbstractBinaryMemcacheMessage
         super.touch(hint);
         return this;
     }
+
+    /**
+     * Copies special metadata hold by this instance to the provided instance
+     *
+     * @param dst The instance where to copy the metadata of this instance to
+     */
+    void copyMeta(DefaultBinaryMemcacheResponse dst) {
+        super.copyMeta(dst);
+        dst.status = status;
+    }
 }

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

@@ -102,7 +102,7 @@ public class DefaultFullBinaryMemcacheRequest extends DefaultBinaryMemcacheReque
         if (extras != null) {
             extras = extras.copy();
         }
-        return new DefaultFullBinaryMemcacheRequest(key, extras, content().copy());
+        return newInstance(key, extras, content().copy());
     }
 
     @Override
@@ -115,7 +115,7 @@ public class DefaultFullBinaryMemcacheRequest extends DefaultBinaryMemcacheReque
         if (extras != null) {
             extras = extras.duplicate();
         }
-        return new DefaultFullBinaryMemcacheRequest(key, extras, content().duplicate());
+        return newInstance(key, extras, content().duplicate());
     }
 
     @Override
@@ -133,6 +133,12 @@ public class DefaultFullBinaryMemcacheRequest extends DefaultBinaryMemcacheReque
         if (extras != null) {
             extras = extras.retainedDuplicate();
         }
-        return new DefaultFullBinaryMemcacheRequest(key, extras, content);
+        return newInstance(key, extras, content);
+    }
+
+    private DefaultFullBinaryMemcacheRequest newInstance(ByteBuf key, ByteBuf extras, ByteBuf content) {
+        DefaultFullBinaryMemcacheRequest newInstance = new DefaultFullBinaryMemcacheRequest(key, extras, content);
+        copyMeta(newInstance);
+        return newInstance;
     }
 }

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

@@ -102,7 +102,7 @@ public class DefaultFullBinaryMemcacheResponse extends DefaultBinaryMemcacheResp
         if (extras != null) {
             extras = extras.copy();
         }
-        return new DefaultFullBinaryMemcacheResponse(key, extras, content().copy());
+        return newInstance(key, extras, content().copy());
     }
 
     @Override
@@ -115,7 +115,7 @@ public class DefaultFullBinaryMemcacheResponse extends DefaultBinaryMemcacheResp
         if (extras != null) {
             extras = extras.duplicate();
         }
-        return new DefaultFullBinaryMemcacheResponse(key, extras, content().duplicate());
+        return newInstance(key, extras, content().duplicate());
     }
 
     @Override
@@ -133,6 +133,12 @@ public class DefaultFullBinaryMemcacheResponse extends DefaultBinaryMemcacheResp
         if (extras != null) {
             extras = extras.retainedDuplicate();
         }
-        return new DefaultFullBinaryMemcacheResponse(key, extras, content);
+        return newInstance(key, extras, content);
+    }
+
+    private FullBinaryMemcacheResponse newInstance(ByteBuf key, ByteBuf extras, ByteBuf content) {
+        DefaultFullBinaryMemcacheResponse newInstance = new DefaultFullBinaryMemcacheResponse(key, extras, content);
+        copyMeta(newInstance);
+        return newInstance;
     }
 }

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

@@ -0,0 +1,99 @@
+/*
+ * Copyright 2019 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.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+
+public class DefaultFullBinaryMemcacheRequestTest {
+
+    private DefaultFullBinaryMemcacheRequest request;
+
+    @Before
+    public void setUp() {
+        request = new DefaultFullBinaryMemcacheRequest(
+                Unpooled.copiedBuffer("key", CharsetUtil.UTF_8),
+                Unpooled.wrappedBuffer(new byte[]{1, 3, 4, 9}),
+                Unpooled.copiedBuffer("some value", CharsetUtil.UTF_8));
+        request.setReserved((short) 534);
+        request.setMagic((byte) 0x03);
+        request.setOpcode((byte) 0x02);
+        request.setKeyLength((short) 32);
+        request.setExtrasLength((byte) 34);
+        request.setDataType((byte) 43);
+        request.setTotalBodyLength(345);
+        request.setOpaque(3);
+        request.setCas(345345L);
+    }
+
+    @Test
+    public void fullCopy() {
+        FullBinaryMemcacheRequest newInstance = request.copy();
+        try {
+            assertCopy(request, request.content(), newInstance);
+        } finally {
+            request.release();
+            newInstance.release();
+        }
+    }
+
+    @Test
+    public void fullDuplicate() {
+        FullBinaryMemcacheRequest newInstance = request.duplicate();
+        try {
+            assertCopy(request, request.content(), newInstance);
+        } finally {
+            request.release();
+        }
+    }
+
+    @Test
+    public void fullReplace() {
+        ByteBuf newContent = Unpooled.copiedBuffer("new value", CharsetUtil.UTF_8);
+        FullBinaryMemcacheRequest newInstance = request.replace(newContent);
+        try {
+            assertCopy(request, newContent, newInstance);
+        } finally {
+            request.release();
+            newInstance.release();
+        }
+    }
+
+    private void assertCopy(FullBinaryMemcacheRequest expected, ByteBuf expectedContent,
+                            FullBinaryMemcacheRequest actual) {
+        assertNotSame(expected, actual);
+
+        assertEquals(expected.key(), actual.key());
+        assertEquals(expected.extras(), actual.extras());
+        assertEquals(expectedContent, actual.content());
+
+        assertEquals(expected.reserved(), actual.reserved());
+        assertEquals(expected.magic(), actual.magic());
+        assertEquals(expected.opcode(), actual.opcode());
+        assertEquals(expected.keyLength(), actual.keyLength());
+        assertEquals(expected.extrasLength(), actual.extrasLength());
+        assertEquals(expected.dataType(), actual.dataType());
+        assertEquals(expected.totalBodyLength(), actual.totalBodyLength());
+        assertEquals(expected.opaque(), actual.opaque());
+        assertEquals(expected.cas(), actual.cas());
+    }
+}

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

@@ -0,0 +1,98 @@
+/*
+ * Copyright 2019 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.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+
+public class DefaultFullBinaryMemcacheResponseTest {
+
+    private DefaultFullBinaryMemcacheResponse response;
+
+    @Before
+    public void setUp() {
+        response = new DefaultFullBinaryMemcacheResponse(
+                Unpooled.copiedBuffer("key", CharsetUtil.UTF_8),
+                Unpooled.wrappedBuffer(new byte[]{1, 3, 4, 9}),
+                Unpooled.copiedBuffer("some value", CharsetUtil.UTF_8));
+        response.setStatus((short) 1);
+        response.setMagic((byte) 0x03);
+        response.setOpcode((byte) 0x02);
+        response.setKeyLength((short) 32);
+        response.setExtrasLength((byte) 34);
+        response.setDataType((byte) 43);
+        response.setTotalBodyLength(345);
+        response.setOpaque(3);
+        response.setCas(345345L);
+    }
+
+    @Test
+    public void fullCopy() {
+        FullBinaryMemcacheResponse newInstance = response.copy();
+        try {
+            assertResponseEquals(response, response.content(), newInstance);
+        } finally {
+            response.release();
+            newInstance.release();
+        }
+    }
+
+    @Test
+    public void fullDuplicate() {
+        try {
+            assertResponseEquals(response, response.content(), response.duplicate());
+        } finally {
+            response.release();
+        }
+    }
+
+    @Test
+    public void fullReplace() {
+        ByteBuf newContent = Unpooled.copiedBuffer("new value", CharsetUtil.UTF_8);
+        FullBinaryMemcacheResponse newInstance = response.replace(newContent);
+        try {
+            assertResponseEquals(response, newContent, newInstance);
+        } finally {
+            response.release();
+            newInstance.release();
+        }
+    }
+
+    private void assertResponseEquals(FullBinaryMemcacheResponse expected, ByteBuf expectedContent,
+                                      FullBinaryMemcacheResponse actual) {
+        assertNotSame(expected, actual);
+
+        assertEquals(expected.key(), actual.key());
+        assertEquals(expected.extras(), actual.extras());
+        assertEquals(expectedContent, actual.content());
+
+        assertEquals(expected.status(), actual.status());
+        assertEquals(expected.magic(), actual.magic());
+        assertEquals(expected.opcode(), actual.opcode());
+        assertEquals(expected.keyLength(), actual.keyLength());
+        assertEquals(expected.extrasLength(), actual.extrasLength());
+        assertEquals(expected.dataType(), actual.dataType());
+        assertEquals(expected.totalBodyLength(), actual.totalBodyLength());
+        assertEquals(expected.opaque(), actual.opaque());
+        assertEquals(expected.cas(), actual.cas());
+    }
+}