From: Robert Varga <robert.varga@pantheon.tech>
Date: Fri, 29 Jul 2022 15:56:58 +0000 (+0200)
Subject: Deprecate DEFAULT_MAXIMUM_CHUNK_SIZE
X-Git-Tag: v4.0.1~5
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F19%2F101919%2F3;p=netconf.git

Deprecate DEFAULT_MAXIMUM_CHUNK_SIZE

This is an artificial global limit. We provide the ability to specify
this limit on a per-instance basis and callers should be taking
advantage of that instead.

Change-Id: I3abbed74c37b4f9a9e820adf0c38fba160a8dc88
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---

diff --git a/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/MessageParserTest.java b/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/MessageParserTest.java
index 88f4f11337..06b76a213d 100644
--- a/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/MessageParserTest.java
+++ b/netconf/netconf-impl/src/test/java/org/opendaylight/netconf/impl/MessageParserTest.java
@@ -5,15 +5,12 @@
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
  * and is available at http://www.eclipse.org/legal/epl-v10.html
  */
-
 package org.opendaylight.netconf.impl;
 
 import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
@@ -35,7 +32,6 @@ import org.opendaylight.netconf.util.messages.FramingMechanism;
 import org.opendaylight.netconf.util.test.XmlFileLoader;
 
 public class MessageParserTest {
-
     private NetconfMessage msg;
 
     @Before
@@ -48,13 +44,12 @@ public class MessageParserTest {
         EmbeddedChannel testChunkChannel = new EmbeddedChannel(
                 FramingMechanismHandlerFactory.createHandler(FramingMechanism.CHUNK),
                 new NetconfMessageToXMLEncoder(),
-
-                new NetconfChunkAggregator(),
+                new NetconfChunkAggregator(ChunkedFramingMechanismEncoder.MAX_CHUNK_SIZE),
                 new NetconfXMLToMessageDecoder());
 
         testChunkChannel.writeOutbound(msg);
         Queue<Object> messages = testChunkChannel.outboundMessages();
-        assertFalse(messages.isEmpty());
+        assertEquals(1, messages.size());
 
         final NetconfMessageToXMLEncoder enc = new NetconfMessageToXMLEncoder();
         final ByteBuf out = Unpooled.buffer();
@@ -83,7 +78,7 @@ public class MessageParserTest {
 
             testChunkChannel.writeInbound(recievedOutbound);
         }
-        assertTrue(messages.isEmpty());
+        assertEquals(0, messages.size());
 
         NetconfMessage receivedMessage = testChunkChannel.readInbound();
         assertNotNull(receivedMessage);
diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
index 6ee1c8b57c..3566bf4bb1 100644
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
@@ -194,7 +194,7 @@ public abstract class AbstractNetconfSessionNegotiator<S extends AbstractNetconf
         replaceChannelHandler(channel, AbstractChannelInitializer.NETCONF_MESSAGE_FRAME_ENCODER,
                 FramingMechanismHandlerFactory.createHandler(FramingMechanism.CHUNK));
         replaceChannelHandler(channel, AbstractChannelInitializer.NETCONF_MESSAGE_AGGREGATOR,
-                new NetconfChunkAggregator());
+                new NetconfChunkAggregator(DEFAULT_MAXIMUM_INCOMING_CHUNK_SIZE));
     }
 
     private boolean shouldUseChunkFraming(final Document doc) {
diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregator.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregator.java
index cab1d741b1..49e6a89dfd 100644
--- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregator.java
+++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregator.java
@@ -27,6 +27,7 @@ public class NetconfChunkAggregator extends ByteToMessageDecoder {
     private static final String GOT_PARAM_WHILE_WAITING_FOR_PARAM_PARAM_PARAM =
         "Got byte {} while waiting for {}-{}-{}";
 
+    @Deprecated(since = "4.0.1", forRemoval = true)
     public static final @NonNegative int DEFAULT_MAXIMUM_CHUNK_SIZE =
         AbstractNetconfSessionNegotiator.DEFAULT_MAXIMUM_INCOMING_CHUNK_SIZE;
 
@@ -49,7 +50,10 @@ public class NetconfChunkAggregator extends ByteToMessageDecoder {
 
     /**
      * Construct an instance with maximum chunk size set to {@link #DEFAULT_MAXIMUM_CHUNK_SIZE}.
+     *
+     * @deprecated Prefer {@link #NetconfChunkAggregator(int)} for fine-grained control.
      */
+    @Deprecated(since = "4.0.1", forRemoval = true)
     public NetconfChunkAggregator() {
         this(DEFAULT_MAXIMUM_CHUNK_SIZE);
     }
diff --git a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregatorTest.java b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregatorTest.java
index 5d692b2fcf..9c5a83e625 100644
--- a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregatorTest.java
+++ b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/NetconfChunkAggregatorTest.java
@@ -13,12 +13,9 @@ import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.List;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 public class NetconfChunkAggregatorTest {
-
     private static final String CHUNKED_MESSAGE = "\n#4\n"
             + "<rpc"
             + "\n#18\n"
@@ -29,36 +26,31 @@ public class NetconfChunkAggregatorTest {
             + "</rpc>"
             + "\n##\n";
 
-    public static final String EXPECTED_MESSAGE = "<rpc message-id=\"102\"\n"
+    private static final String EXPECTED_MESSAGE = "<rpc message-id=\"102\"\n"
             + "     xmlns=\"urn:ietf:params:xml:ns:netconf:base:1.0\">\n"
             + "  <close-session/>\n"
             + "</rpc>";
 
     private static final String CHUNKED_MESSAGE_ONE = "\n#101\n" + EXPECTED_MESSAGE + "\n##\n";
 
-    private static NetconfChunkAggregator agr;
-
-    @BeforeClass
-    public static void setUp() throws Exception {
-        agr = new NetconfChunkAggregator();
-    }
+    private final NetconfChunkAggregator agr = new NetconfChunkAggregator(4096);
 
     @Test
     public void testMultipleChunks() throws Exception {
-        final List<Object> output = new ArrayList<>();
-        final ByteBuf input = Unpooled.copiedBuffer(CHUNKED_MESSAGE.getBytes(StandardCharsets.UTF_8));
+        final var output = new ArrayList<>();
+        final var input = Unpooled.copiedBuffer(CHUNKED_MESSAGE.getBytes(StandardCharsets.UTF_8));
         agr.decode(null, input, output);
 
         assertEquals(1, output.size());
-        final ByteBuf chunk = (ByteBuf) output.get(0);
+        final var chunk = (ByteBuf) output.get(0);
 
         assertEquals(EXPECTED_MESSAGE, chunk.toString(StandardCharsets.UTF_8));
     }
 
     @Test
     public void testOneChunks() throws Exception {
-        final List<Object> output = new ArrayList<>();
-        final ByteBuf input = Unpooled.copiedBuffer(CHUNKED_MESSAGE_ONE.getBytes(StandardCharsets.UTF_8));
+        final var output = new ArrayList<>();
+        final var input = Unpooled.copiedBuffer(CHUNKED_MESSAGE_ONE.getBytes(StandardCharsets.UTF_8));
         agr.decode(null, input, output);
 
         assertEquals(1, output.size());