From 3e64cdb68fe925d0180f106946b4d9351499d5d3 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 23 Oct 2023 20:00:01 +0200 Subject: [PATCH] Refactor FramingMechanismHandlerFactory 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 --- .../nettyutil/AbstractChannelInitializer.java | 6 ++--- .../AbstractNetconfSessionNegotiator.java | 7 +++--- .../ChunkedFramingMechanismEncoder.java | 21 ++++++++-------- .../handler/EOMFramingMechanismEncoder.java | 10 +++++--- ...tory.java => FramingMechanismEncoder.java} | 23 ++++++++++++----- .../AbstractNetconfSessionNegotiatorTest.java | 5 +--- .../netconf/nettyutil/Netconf539Test.java | 6 ++--- .../handler/FramingMechanismEncoderTest.java | 21 ++++++++++++++++ .../FramingMechanismHandlerFactoryTest.java | 25 ------------------- .../netconf/server/MessageParserTest.java | 11 ++++---- 10 files changed, 68 insertions(+), 67 deletions(-) rename netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/{FramingMechanismHandlerFactory.java => FramingMechanismEncoder.java} (51%) create mode 100644 netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoderTest.java delete mode 100644 netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactoryTest.java diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractChannelInitializer.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractChannelInitializer.java index 1a7df7b715..c55f0b2973 100644 --- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractChannelInitializer.java +++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/AbstractChannelInitializer.java @@ -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 { public void initialize(final Channel ch, final Promise 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); 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 a2af2f3103..a5d65a37c9 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 @@ -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 { +/** + * 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= 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 { diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/EOMFramingMechanismEncoder.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/EOMFramingMechanismEncoder.java index 74009a7301..dafec95d02 100644 --- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/EOMFramingMechanismEncoder.java +++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/EOMFramingMechanismEncoder.java @@ -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 { +/** + * 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); } diff --git a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactory.java b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoder.java similarity index 51% rename from netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactory.java rename to netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoder.java index 9294853d95..66cf5a5893 100644 --- a/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactory.java +++ b/netconf/netconf-netty-util/src/main/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoder.java @@ -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 + 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 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(); diff --git a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiatorTest.java b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiatorTest.java index 7f43857b38..4e6952f909 100644 --- a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiatorTest.java +++ b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/AbstractNetconfSessionNegotiatorTest.java @@ -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()); diff --git a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/Netconf539Test.java b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/Netconf539Test.java index cd593030ac..c6353662f9 100644 --- a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/Netconf539Test.java +++ b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/Netconf539Test.java @@ -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 index 0000000000..fc30f9720d --- /dev/null +++ b/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismEncoderTest.java @@ -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 index 8e88544e27..0000000000 --- a/netconf/netconf-netty-util/src/test/java/org/opendaylight/netconf/nettyutil/handler/FramingMechanismHandlerFactoryTest.java +++ /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 diff --git a/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/MessageParserTest.java b/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/MessageParserTest.java index eda64ce21a..e9bcb31739 100644 --- a/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/MessageParserTest.java +++ b/protocol/netconf-server/src/test/java/org/opendaylight/netconf/server/MessageParserTest.java @@ -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); -- 2.36.6