Refactor FramingMechanismHandlerFactory 01/108601/4
authorRobert Varga <robert.varga@pantheon.tech>
Mon, 23 Oct 2023 18:00:01 +0000 (20:00 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Mon, 23 Oct 2023 18:22:50 +0000 (20:22 +0200)
Rename FramingMechanismHandlerFactory to FramingMechanismEncoder and
make it act as abstract superclass to encoders. Also rename
createHandler() to simple of().

Furthermore update users to directly pick encoders, as these are always
compile-time constants.

Change-Id: Idc54c4ccbb3e3706a5f283f966826c5a4997e95d
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractChannelInitializer.java
netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiator.java
netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/ChunkedFramingMechanismEncoder.java
netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/EOMFramingMechanismEncoder.java
netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoder.java [moved from netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactory.java with 51% similarity]
netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiatorTest.java
netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/Netconf539Test.java
netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoderTest.java [new file with mode: 0644]
netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactoryTest.java [deleted file]
protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/MessageParserTest.java

index 1a7df7b715824553268a693478277c03f92da3b3..c55f0b2973a78ecd7b541634924ca75453aca5c9 100644 (file)
@@ -10,8 +10,7 @@ package org.opendaylight.netconf.nettyutil;
 import io.netty.channel.Channel;
 import io.netty.util.concurrent.Promise;
 import org.opendaylight.netconf.api.NetconfSession;
-import org.opendaylight.netconf.api.messages.FramingMechanism;
-import org.opendaylight.netconf.nettyutil.handler.FramingMechanismHandlerFactory;
+import org.opendaylight.netconf.nettyutil.handler.EOMFramingMechanismEncoder;
 import org.opendaylight.netconf.nettyutil.handler.NetconfEOMAggregator;
 import org.opendaylight.netconf.nettyutil.handler.NetconfHelloMessageToXMLEncoder;
 import org.opendaylight.netconf.nettyutil.handler.NetconfXMLToHelloMessageDecoder;
@@ -26,8 +25,7 @@ public abstract class AbstractChannelInitializer<S extends NetconfSession> {
     public void initialize(final Channel ch, final Promise<S> promise) {
         ch.pipeline().addLast(NETCONF_MESSAGE_AGGREGATOR, new NetconfEOMAggregator());
         initializeMessageDecoder(ch);
-        ch.pipeline().addLast(NETCONF_MESSAGE_FRAME_ENCODER,
-                FramingMechanismHandlerFactory.createHandler(FramingMechanism.EOM));
+        ch.pipeline().addLast(NETCONF_MESSAGE_FRAME_ENCODER, new EOMFramingMechanismEncoder());
         initializeMessageEncoder(ch);
 
         initializeSessionNegotiator(ch, promise);
index a2af2f3103df21a6f31469a4bc386f07b4904c0c..a5d65a37c9959fdf58558a935ad5ced2d44d2de4 100644 (file)
@@ -31,11 +31,10 @@ import org.opendaylight.netconf.api.CapabilityURN;
 import org.opendaylight.netconf.api.NamespaceURN;
 import org.opendaylight.netconf.api.NetconfDocumentedException;
 import org.opendaylight.netconf.api.NetconfSessionListener;
-import org.opendaylight.netconf.api.messages.FramingMechanism;
 import org.opendaylight.netconf.api.messages.HelloMessage;
 import org.opendaylight.netconf.api.messages.NetconfMessage;
 import org.opendaylight.netconf.api.xml.XmlNetconfConstants;
-import org.opendaylight.netconf.nettyutil.handler.FramingMechanismHandlerFactory;
+import org.opendaylight.netconf.nettyutil.handler.ChunkedFramingMechanismEncoder;
 import org.opendaylight.netconf.nettyutil.handler.NetconfChunkAggregator;
 import org.opendaylight.netconf.nettyutil.handler.NetconfMessageToXMLEncoder;
 import org.opendaylight.netconf.nettyutil.handler.NetconfXMLToHelloMessageDecoder;
@@ -262,9 +261,9 @@ public abstract class AbstractNetconfSessionNegotiator<S extends AbstractNetconf
      */
     private void insertChunkFramingToPipeline() {
         replaceChannelHandler(channel, AbstractChannelInitializer.NETCONF_MESSAGE_FRAME_ENCODER,
-                FramingMechanismHandlerFactory.createHandler(FramingMechanism.CHUNK));
+            new ChunkedFramingMechanismEncoder());
         replaceChannelHandler(channel, AbstractChannelInitializer.NETCONF_MESSAGE_AGGREGATOR,
-                new NetconfChunkAggregator(maximumIncomingChunkSize));
+            new NetconfChunkAggregator(maximumIncomingChunkSize));
     }
 
     private boolean shouldUseChunkFraming(final Document doc) {
index f2592e42304fa62bc1fc9bcf6b149a6c7a7d6364..0e059af5fb233ad38fc3cabc7224a22644814174 100644 (file)
@@ -5,16 +5,17 @@
  * 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.nettyutil.handler;
 
-import com.google.common.base.Preconditions;
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
-import io.netty.handler.codec.MessageToByteEncoder;
 import java.nio.charset.StandardCharsets;
+import org.opendaylight.netconf.api.messages.FramingMechanism;
 
-public class ChunkedFramingMechanismEncoder extends MessageToByteEncoder<ByteBuf> {
+/**
+ * A {@link FramingMechanismEncoder} handling {@link FramingMechanism#CHUNK}.
+ */
+public final class ChunkedFramingMechanismEncoder extends FramingMechanismEncoder {
     public static final int DEFAULT_CHUNK_SIZE = 8192;
     public static final int MIN_CHUNK_SIZE = 128;
     public static final int MAX_CHUNK_SIZE = 16 * 1024 * 1024;
@@ -26,15 +27,15 @@ public class ChunkedFramingMechanismEncoder extends MessageToByteEncoder<ByteBuf
     }
 
     public ChunkedFramingMechanismEncoder(final int chunkSize) {
-        Preconditions.checkArgument(chunkSize >= MIN_CHUNK_SIZE && chunkSize <= MAX_CHUNK_SIZE,
-                "Unsupported chunk size %s", chunkSize);
+        if (chunkSize < MIN_CHUNK_SIZE) {
+            throw new IllegalArgumentException(chunkSize + " is lower than minimum supported " + MIN_CHUNK_SIZE);
+        }
+        if (chunkSize > MAX_CHUNK_SIZE) {
+            throw new IllegalArgumentException(chunkSize + " is lower than maximum supported " + MAX_CHUNK_SIZE);
+        }
         this.chunkSize = chunkSize;
     }
 
-    public final int getChunkSize() {
-        return chunkSize;
-    }
-
     @Override
     protected void encode(final ChannelHandlerContext ctx, final ByteBuf msg, final ByteBuf out)  {
         do {
index 74009a7301b87b9546ba26793ce89c99bb9b854d..dafec95d02118c40ac6ab933a9894aaa869ebe2e 100644 (file)
@@ -5,16 +5,18 @@
  * 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.nettyutil.handler;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
-import io.netty.handler.codec.MessageToByteEncoder;
+import org.opendaylight.netconf.api.messages.FramingMechanism;
 
-public class EOMFramingMechanismEncoder extends MessageToByteEncoder<ByteBuf> {
+/**
+ * A {@link FramingMechanismEncoder} handling {@link FramingMechanism#EOM}.
+ */
+public final class EOMFramingMechanismEncoder extends FramingMechanismEncoder {
     @Override
-    protected void encode(ChannelHandlerContext ctx, ByteBuf msg, ByteBuf out) {
+    protected void encode(final ChannelHandlerContext ctx, final ByteBuf msg, final ByteBuf out) {
         out.writeBytes(msg);
         out.writeBytes(MessageParts.END_OF_MESSAGE);
     }
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013 Cisco Systems, Inc. and others.  All rights reserved.
+ * Copyright (c) 2023 PANTHEON.tech, s.r.o. and others.  All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
@@ -9,18 +9,29 @@ package org.opendaylight.netconf.nettyutil.handler;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.handler.codec.MessageToByteEncoder;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.netconf.api.messages.FramingMechanism;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public final class FramingMechanismHandlerFactory {
-    private static final Logger LOG = LoggerFactory.getLogger(FramingMechanismHandlerFactory.class);
+/**
+ * An channel handler framing outbound messages into specified framing.
+ */
+public abstract sealed class FramingMechanismEncoder extends MessageToByteEncoder<ByteBuf>
+        permits ChunkedFramingMechanismEncoder, EOMFramingMechanismEncoder {
+    private static final Logger LOG = LoggerFactory.getLogger(FramingMechanismEncoder.class);
 
-    private FramingMechanismHandlerFactory() {
-        // not called - private constructor for utility class
+    FramingMechanismEncoder() {
+        // Hidden on purpose
     }
 
-    public static MessageToByteEncoder<ByteBuf> createHandler(final FramingMechanism framingMechanism) {
+    /**
+     * Return a {@link FramingMechanismEncoder} for specified {@link FramingMechanism}.
+     *
+     * @param framingMechanism Desired {@link FramingMechanism}
+     * @return A {@link FramingMechanismEncoder}
+     */
+    public static final @NonNull FramingMechanismEncoder of(final FramingMechanism framingMechanism) {
         LOG.debug("{} framing mechanism was selected.", framingMechanism);
         return switch (framingMechanism) {
             case CHUNK -> new ChunkedFramingMechanismEncoder();
index 7f43857b38313805894ad604dbc4a04892432093..4e6952f90926215cb483291cb740e34d7e0e3323 100644 (file)
@@ -49,12 +49,10 @@ import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.opendaylight.netconf.api.CapabilityURN;
 import org.opendaylight.netconf.api.NetconfSessionListener;
-import org.opendaylight.netconf.api.messages.FramingMechanism;
 import org.opendaylight.netconf.api.messages.HelloMessage;
 import org.opendaylight.netconf.api.xml.XmlUtil;
 import org.opendaylight.netconf.nettyutil.handler.ChunkedFramingMechanismEncoder;
 import org.opendaylight.netconf.nettyutil.handler.EOMFramingMechanismEncoder;
-import org.opendaylight.netconf.nettyutil.handler.FramingMechanismHandlerFactory;
 import org.opendaylight.netconf.nettyutil.handler.NetconfChunkAggregator;
 import org.opendaylight.netconf.nettyutil.handler.NetconfEOMAggregator;
 import org.opendaylight.netconf.nettyutil.handler.NetconfXMLToHelloMessageDecoder;
@@ -84,8 +82,7 @@ public class AbstractNetconfSessionNegotiatorTest {
         channel.pipeline().addLast(AbstractChannelInitializer.NETCONF_MESSAGE_ENCODER,
                 new ChannelInboundHandlerAdapter());
         channel.pipeline().addLast(AbstractChannelInitializer.NETCONF_MESSAGE_DECODER, xmlToHello);
-        channel.pipeline().addLast(NETCONF_MESSAGE_FRAME_ENCODER,
-                FramingMechanismHandlerFactory.createHandler(FramingMechanism.EOM));
+        channel.pipeline().addLast(NETCONF_MESSAGE_FRAME_ENCODER, new EOMFramingMechanismEncoder());
         channel.pipeline().addLast(NETCONF_MESSAGE_AGGREGATOR, new NetconfEOMAggregator());
         hello = HelloMessage.createClientHello(Set.of(), Optional.empty());
         helloBase11 = HelloMessage.createClientHello(Set.of(CapabilityURN.BASE_1_1), Optional.empty());
index cd593030ac5d9353d046bcee2f6029c65a5d19f3..c6353662f9765c9c4706be9f99e034cde9b79da1 100644 (file)
@@ -25,10 +25,9 @@ import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.opendaylight.netconf.api.CapabilityURN;
 import org.opendaylight.netconf.api.NetconfSessionListener;
-import org.opendaylight.netconf.api.messages.FramingMechanism;
 import org.opendaylight.netconf.api.messages.HelloMessage;
 import org.opendaylight.netconf.nettyutil.handler.ChunkedFramingMechanismEncoder;
-import org.opendaylight.netconf.nettyutil.handler.FramingMechanismHandlerFactory;
+import org.opendaylight.netconf.nettyutil.handler.EOMFramingMechanismEncoder;
 import org.opendaylight.netconf.nettyutil.handler.NetconfChunkAggregator;
 import org.opendaylight.netconf.nettyutil.handler.NetconfEOMAggregator;
 import org.opendaylight.netconf.nettyutil.handler.NetconfXMLToHelloMessageDecoder;
@@ -52,8 +51,7 @@ public class Netconf539Test {
             new ChannelInboundHandlerAdapter());
         channel.pipeline().addLast(AbstractChannelInitializer.NETCONF_MESSAGE_DECODER,
             new NetconfXMLToHelloMessageDecoder());
-        channel.pipeline().addLast(NETCONF_MESSAGE_FRAME_ENCODER,
-            FramingMechanismHandlerFactory.createHandler(FramingMechanism.EOM));
+        channel.pipeline().addLast(NETCONF_MESSAGE_FRAME_ENCODER, new EOMFramingMechanismEncoder());
         channel.pipeline().addLast(NETCONF_MESSAGE_AGGREGATOR, new NetconfEOMAggregator());
         final HelloMessage serverHello = HelloMessage.createClientHello(Set.of(CapabilityURN.BASE_1_1),
             Optional.empty());
diff --git a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoderTest.java b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoderTest.java
new file mode 100644 (file)
index 0000000..fc30f97
--- /dev/null
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2014 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * 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.nettyutil.handler;
+
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+
+import org.junit.jupiter.api.Test;
+import org.opendaylight.netconf.api.messages.FramingMechanism;
+
+class FramingMechanismEncoderTest {
+    @Test
+    void testCreate() {
+        assertInstanceOf(ChunkedFramingMechanismEncoder.class, FramingMechanismEncoder.of(FramingMechanism.CHUNK));
+        assertInstanceOf(EOMFramingMechanismEncoder.class, FramingMechanismEncoder.of(FramingMechanism.EOM));
+    }
+}
\ No newline at end of file
diff --git a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactoryTest.java b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactoryTest.java
deleted file mode 100644 (file)
index 8e88544..0000000
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * Copyright (c) 2014 Cisco Systems, Inc. and others.  All rights reserved.
- *
- * This program and the accompanying materials are made available under the
- * 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.nettyutil.handler;
-
-import org.hamcrest.CoreMatchers;
-import org.hamcrest.MatcherAssert;
-import org.junit.Test;
-import org.opendaylight.netconf.api.messages.FramingMechanism;
-
-public class FramingMechanismHandlerFactoryTest {
-    @Test
-    public void testCreate() throws Exception {
-        MatcherAssert.assertThat(FramingMechanismHandlerFactory
-                .createHandler(FramingMechanism.CHUNK), CoreMatchers
-                .instanceOf(ChunkedFramingMechanismEncoder.class));
-        MatcherAssert.assertThat(FramingMechanismHandlerFactory
-                .createHandler(FramingMechanism.EOM), CoreMatchers
-                .instanceOf(EOMFramingMechanismEncoder.class));
-    }
-}
\ No newline at end of file
index eda64ce21a182a6db745f9277966bce98c48b514..e9bcb317396350c8142cf0f451bb1652b1c1bde8 100644 (file)
@@ -19,10 +19,9 @@ import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import org.junit.Before;
 import org.junit.Test;
-import org.opendaylight.netconf.api.messages.FramingMechanism;
 import org.opendaylight.netconf.api.messages.NetconfMessage;
 import org.opendaylight.netconf.nettyutil.handler.ChunkedFramingMechanismEncoder;
-import org.opendaylight.netconf.nettyutil.handler.FramingMechanismHandlerFactory;
+import org.opendaylight.netconf.nettyutil.handler.EOMFramingMechanismEncoder;
 import org.opendaylight.netconf.nettyutil.handler.NetconfChunkAggregator;
 import org.opendaylight.netconf.nettyutil.handler.NetconfEOMAggregator;
 import org.opendaylight.netconf.nettyutil.handler.NetconfMessageToXMLEncoder;
@@ -41,7 +40,7 @@ public class MessageParserTest {
     @Test
     public void testChunkedFramingMechanismOnPipeline() throws Exception {
         final var testChunkChannel = new EmbeddedChannel(
-                FramingMechanismHandlerFactory.createHandler(FramingMechanism.CHUNK),
+                new ChunkedFramingMechanismEncoder(),
                 new NetconfMessageToXMLEncoder(),
                 new NetconfChunkAggregator(ChunkedFramingMechanismEncoder.MAX_CHUNK_SIZE),
                 new NetconfXMLToMessageDecoder());
@@ -60,7 +59,7 @@ public class MessageParserTest {
             chunkCount++;
         }
 
-        final var endOfChunkBytes = FramingMechanism.CHUNK_END_STR.getBytes(StandardCharsets.US_ASCII);
+        final var endOfChunkBytes = "\n##\n".getBytes(StandardCharsets.US_ASCII);
         for (int i = 1; i <= chunkCount; i++) {
             final var recievedOutbound = (ByteBuf) messages.poll();
             int exptHeaderLength = ChunkedFramingMechanismEncoder.DEFAULT_CHUNK_SIZE;
@@ -93,13 +92,13 @@ public class MessageParserTest {
     @Test
     public void testEOMFramingMechanismOnPipeline() throws Exception {
         final var testChunkChannel = new EmbeddedChannel(
-                FramingMechanismHandlerFactory.createHandler(FramingMechanism.EOM),
+                new EOMFramingMechanismEncoder(),
                 new NetconfMessageToXMLEncoder(), new NetconfEOMAggregator(), new NetconfXMLToMessageDecoder());
 
         testChunkChannel.writeOutbound(msg);
         final ByteBuf recievedOutbound = testChunkChannel.readOutbound();
 
-        final var endOfMsgBytes = FramingMechanism.EOM_STR.getBytes(StandardCharsets.US_ASCII);
+        final var endOfMsgBytes = "]]>]]>".getBytes(StandardCharsets.US_ASCII);
         final var eom = new byte[endOfMsgBytes.length];
         recievedOutbound.getBytes(recievedOutbound.readableBytes() - endOfMsgBytes.length, eom);
         assertArrayEquals(endOfMsgBytes, eom);