From 1dca383a7e732a5a1b2ad78f2a73b2c6cf97edc1 Mon Sep 17 00:00:00 2001 From: Dana Kutenicsova Date: Mon, 19 Aug 2013 16:25:33 +0200 Subject: [PATCH] Implemented closing operations for PCEP Netty. Change-Id: If5bb5788ad315f99907ab69d999c01b923ad2017 Signed-off-by: Dana Kutenicsova --- .../protocol/bgp/rib/impl/BGPSessionImpl.java | 6 -- .../protocol/framework/Dispatcher.java | 5 +- .../protocol/framework/DispatcherImpl.java | 44 ++++++++++++--- .../framework/ProtocolMessageDecoder.java | 6 +- .../framework/ProtocolMessageEncoder.java | 4 +- .../protocol/framework/ProtocolServer.java | 56 ++++++++++++------- .../protocol/framework/ProtocolSession.java | 8 --- .../protocol/framework/ServerTest.java | 29 ++++------ .../protocol/framework/Session.java | 5 -- .../protocol/framework/SimpleSession.java | 5 -- framework/src/test/resources/logback-test.xml | 2 +- .../protocol/pcep/PCEPSession.java | 22 ++++---- .../protocol/pcep/PCEPSessionListener.java | 26 ++++----- pcep/impl/pom.xml | 5 -- .../pcep/impl/PCEPDispatcherImpl.java | 8 +-- .../pcep/impl/PCEPMessageFactory.java | 2 +- .../protocol/pcep/impl/PCEPSessionImpl.java | 50 +++++++---------- .../protocol/pcep/impl/MockPCE.java | 31 +++++----- .../pcep/impl/SimpleSessionListener.java | 24 ++++---- pcep/testtool/pom.xml | 5 -- .../pcep/testtool/SimpleSessionListener.java | 26 ++++----- .../pcep/testtool/TestingSessionListener.java | 11 +++- 22 files changed, 179 insertions(+), 201 deletions(-) diff --git a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java index a2bf6a5d5b..9f2c51f1da 100644 --- a/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java +++ b/bgp/rib-impl/src/main/java/org/opendaylight/protocol/bgp/rib/impl/BGPSessionImpl.java @@ -224,12 +224,6 @@ class BGPSessionImpl implements BGPSession, ProtocolSession { return this.parser; } - @Override - public void onConnectionFailed(final IOException e) { - logger.info("Connection failed before finishing: {}", e.getMessage(), e); - this.listener.onSessionDown(this, e); - } - @Override public int maximumMessageSize() { return 4096; diff --git a/framework/src/main/java/org/opendaylight/protocol/framework/Dispatcher.java b/framework/src/main/java/org/opendaylight/protocol/framework/Dispatcher.java index 4dbd80daed..2825d319c6 100644 --- a/framework/src/main/java/org/opendaylight/protocol/framework/Dispatcher.java +++ b/framework/src/main/java/org/opendaylight/protocol/framework/Dispatcher.java @@ -7,8 +7,6 @@ */ package org.opendaylight.protocol.framework; -import io.netty.util.concurrent.Future; - import java.io.IOException; import java.net.InetSocketAddress; @@ -37,6 +35,5 @@ public interface Dispatcher { * * @return session associated with this client */ - public Future createClient(final ProtocolConnection connection, final ProtocolSessionFactory sfactory) - throws IOException; + public ProtocolSession createClient(final ProtocolConnection connection, final ProtocolSessionFactory sfactory) throws IOException; } diff --git a/framework/src/main/java/org/opendaylight/protocol/framework/DispatcherImpl.java b/framework/src/main/java/org/opendaylight/protocol/framework/DispatcherImpl.java index d818bfafb8..1b7f69fd1a 100644 --- a/framework/src/main/java/org/opendaylight/protocol/framework/DispatcherImpl.java +++ b/framework/src/main/java/org/opendaylight/protocol/framework/DispatcherImpl.java @@ -9,6 +9,7 @@ package org.opendaylight.protocol.framework; import io.netty.bootstrap.Bootstrap; import io.netty.bootstrap.ServerBootstrap; +import io.netty.channel.Channel; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandler; @@ -20,15 +21,18 @@ import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.channel.socket.nio.NioSocketChannel; import io.netty.util.concurrent.DefaultPromise; -import io.netty.util.concurrent.Future; import java.io.IOException; import java.net.InetSocketAddress; +import java.util.Map; import java.util.Timer; +import java.util.concurrent.ExecutionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.collect.Maps; + /** * Dispatcher class for creating servers and clients. The idea is to first create servers and clients and the run the * start method that will handle sockets in different thread. @@ -122,17 +126,23 @@ public final class DispatcherImpl implements Dispatcher, SessionParent { private final ProtocolMessageFactory messageFactory; + private final Map serverSessions; + + private final Map clientSessions; + public DispatcherImpl(final ProtocolMessageFactory factory) { this.bossGroup = new NioEventLoopGroup(); this.workerGroup = new NioEventLoopGroup(); this.stateTimer = new Timer(); this.messageFactory = factory; + this.clientSessions = Maps.newHashMap(); + this.serverSessions = Maps.newHashMap(); } @Override public ProtocolServer createServer(final InetSocketAddress address, final ProtocolConnectionFactory connectionFactory, final ProtocolSessionFactory sessionFactory) { - final ProtocolServer server = new ProtocolServer(address, connectionFactory, sessionFactory); + final ProtocolServer server = new ProtocolServer(address, connectionFactory, sessionFactory, this); final ServerBootstrap b = new ServerBootstrap(); b.group(this.bossGroup, this.workerGroup); b.channel(NioServerSocketChannel.class); @@ -141,13 +151,14 @@ public final class DispatcherImpl implements Dispatcher, SessionParent { b.childOption(ChannelOption.SO_KEEPALIVE, true); // Bind and start to accept incoming connections. - b.bind(address); - logger.debug("Server {} created.", server); + final ChannelFuture f = b.bind(address); + this.serverSessions.put(server, f.channel()); + logger.debug("Created server {}.", server); return server; } @Override - public Future createClient(final ProtocolConnection connection, final ProtocolSessionFactory sfactory) { + public ProtocolSession createClient(final ProtocolConnection connection, final ProtocolSessionFactory sfactory) { final Bootstrap b = new Bootstrap(); b.group(this.workerGroup); b.channel(NioSocketChannel.class); @@ -169,8 +180,15 @@ public final class DispatcherImpl implements Dispatcher, SessionParent { p.setFailure(cf.cause()); } }); + ProtocolSession s = null; + try { + s = p.get(); + this.clientSessions.put(p.get(), f.channel()); + } catch (InterruptedException | ExecutionException e) { + logger.warn("Client not created. Exception {}.", e.getMessage(), e); + } logger.debug("Client created."); - return p; + return s; } @Override @@ -181,6 +199,18 @@ public final class DispatcherImpl implements Dispatcher, SessionParent { @Override public void onSessionClosed(final ProtocolSession session) { - // TODO Auto-generated method stub + logger.trace("Removing client session: {}", session); + final Channel ch = this.clientSessions.get(session); + ch.close(); + this.clientSessions.remove(session); + logger.debug("Removed client session: {}", session.toString()); + } + + void onServerClosed(final ProtocolServer server) { + logger.trace("Removing server session: {}", server); + final Channel ch = this.serverSessions.get(server); + ch.close(); + this.clientSessions.remove(server); + logger.debug("Removed server session: {}", server.toString()); } } diff --git a/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolMessageDecoder.java b/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolMessageDecoder.java index 26e7a48720..85fdd5753a 100644 --- a/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolMessageDecoder.java +++ b/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolMessageDecoder.java @@ -29,12 +29,16 @@ final class ProtocolMessageDecoder extends ByteToMessageDecoder { @Override protected void decode(final ChannelHandlerContext ctx, final ByteBuf in, final List out) throws Exception { + if (in.readableBytes() == 0) { + logger.debug("No more content in incoming buffer."); + return; + } in.markReaderIndex(); ProtocolMessage msg = null; try { final byte[] bytes = new byte[in.readableBytes()]; - logger.debug("Received to decode: {}", Arrays.toString(bytes)); in.readBytes(bytes); + logger.debug("Received to decode: {}", Arrays.toString(bytes)); msg = this.factory.parse(bytes); } catch (DeserializerException | DocumentedException e) { this.exceptionCaught(ctx, e); diff --git a/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolMessageEncoder.java b/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolMessageEncoder.java index 919caa96d9..80836311c2 100644 --- a/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolMessageEncoder.java +++ b/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolMessageEncoder.java @@ -12,8 +12,6 @@ import io.netty.channel.ChannelHandler.Sharable; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.MessageToByteEncoder; -import java.util.Arrays; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -30,7 +28,7 @@ final class ProtocolMessageEncoder extends MessageToByteEncoder @Override protected void encode(final ChannelHandlerContext ctx, final ProtocolMessage msg, final ByteBuf out) throws Exception { - logger.debug("Sent to encode : {}", Arrays.toString(ctx.channel().pipeline().names().toArray())); + logger.debug("Sent to encode : {}", msg); out.writeBytes(this.factory.put(msg)); } } diff --git a/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolServer.java b/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolServer.java index 903fb83918..b7bca4bd08 100644 --- a/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolServer.java +++ b/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolServer.java @@ -14,7 +14,6 @@ import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.util.HashMap; import java.util.Map; import java.util.Timer; @@ -23,12 +22,13 @@ import org.slf4j.LoggerFactory; import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; +import com.google.common.collect.Maps; /** * Representation of a server, created by {@link Dispatcher}. Should be extended by a protocol specific server * implementation. */ -public class ProtocolServer implements SessionParent { +public final class ProtocolServer implements SessionParent { private static final Logger logger = LoggerFactory.getLogger(ProtocolServer.class); @@ -47,23 +47,24 @@ public class ProtocolServer implements SessionParent { private final Map sessionIds; + private final Dispatcher parent; + /** * Creates a Protocol server. * - * @param dispatcher Dispatcher * @param address address to which this server is bound * @param connectionFactory factory for connection specific properties - * @param channel server socket channel + * @param parent Dispatcher that created this server * @param sessionFactory factory for sessions - * @param inputStreamFactory factory for input streams */ public ProtocolServer(final InetSocketAddress address, final ProtocolConnectionFactory connectionFactory, - final ProtocolSessionFactory sessionFactory) { + final ProtocolSessionFactory sessionFactory, final Dispatcher parent) { this.serverAddress = address; this.sessions = HashBiMap.create(); this.connectionFactory = connectionFactory; this.sessionFactory = sessionFactory; - this.sessionIds = new HashMap(); + this.parent = parent; + this.sessionIds = Maps.newHashMap(); } /** @@ -85,7 +86,7 @@ public class ProtocolServer implements SessionParent { try { session.close(); } catch (final IOException e) { - logger.error("Session {} could not be closed.", session); + logger.error("Could not close session: {}.", session); } } } else { @@ -98,24 +99,16 @@ public class ProtocolServer implements SessionParent { return session; } - /** - * Returns server address. - * - * @return server address - */ - public InetSocketAddress getAddress() { - return this.serverAddress; - } - @Override public synchronized void close() throws IOException { - // TODO: - logger.debug("Server {} closed.", this); + ((DispatcherImpl) this.parent).onServerClosed(this); + logger.debug("Closed server {}.", this); } @Override public synchronized void onSessionClosed(final ProtocolSession session) { this.sessions.inverse().remove(session); // when the session is closed, the key is the instance of the session + logger.debug("Closed session {}.", session); } private static int getNextId(Integer lastId, final int maxId) { @@ -150,4 +143,29 @@ public class ProtocolServer implements SessionParent { public String toString() { return "ProtocolServer [serverAddress=" + this.serverAddress + ", hashCode()=" + hashCode() + "]"; } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((this.serverAddress == null) ? 0 : this.serverAddress.hashCode()); + return result; + } + + @Override + public boolean equals(final Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + final ProtocolServer other = (ProtocolServer) obj; + if (this.serverAddress == null) { + if (other.serverAddress != null) + return false; + } else if (!this.serverAddress.equals(other.serverAddress)) + return false; + return true; + } } diff --git a/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolSession.java b/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolSession.java index 857169305a..0cec78787c 100644 --- a/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolSession.java +++ b/framework/src/main/java/org/opendaylight/protocol/framework/ProtocolSession.java @@ -8,7 +8,6 @@ package org.opendaylight.protocol.framework; import java.io.Closeable; -import java.io.IOException; /** * Protocol Session represents the finite state machine in underlying protocol, including timers and its purpose is to @@ -60,13 +59,6 @@ public interface ProtocolSession extends Closeable { */ public ProtocolMessageFactory getMessageFactory(); - /** - * Session is notified about the connection not being established successfully. - * - * @param e IOException that was the cause of failed connection. - */ - public void onConnectionFailed(final IOException e); - /** * Returns the maximum message size (in bytes) for purposes of dispatcher buffering -- the dispatcher allocates a * buffer this big, and if it gets full without making decoding progress, the dispatcher terminates the session. diff --git a/framework/src/test/java/org/opendaylight/protocol/framework/ServerTest.java b/framework/src/test/java/org/opendaylight/protocol/framework/ServerTest.java index 77b9e0569c..18efc51d0a 100644 --- a/framework/src/test/java/org/opendaylight/protocol/framework/ServerTest.java +++ b/framework/src/test/java/org/opendaylight/protocol/framework/ServerTest.java @@ -8,13 +8,10 @@ package org.opendaylight.protocol.framework; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.io.IOException; -import java.net.ConnectException; import java.net.InetSocketAddress; import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; import org.junit.After; import org.junit.Test; @@ -31,13 +28,13 @@ public class ServerTest { ProtocolServer server = null; + public final InetSocketAddress serverAddress = new InetSocketAddress("127.0.0.5", PORT); + @Test public void testConnectionEstablished() throws Exception { this.dispatcher = new DispatcherImpl(new MessageFactory()); - final InetSocketAddress serverAddress = new InetSocketAddress("127.0.0.3", PORT); - - this.server = this.dispatcher.createServer(serverAddress, new ProtocolConnectionFactory() { + this.server = this.dispatcher.createServer(this.serverAddress, new ProtocolConnectionFactory() { @Override public ProtocolConnection createProtocolConnection(final InetSocketAddress address) { @@ -80,14 +77,14 @@ public class ServerTest { @Override public InetSocketAddress getPeerAddress() { - return new InetSocketAddress("127.0.0.3", PORT); + return ServerTest.this.serverAddress; } @Override public SessionListener getListener() { return ServerTest.this.pce; } - }, new SimpleSessionFactory(MAX_MSGSIZE)).get(); + }, new SimpleSessionFactory(MAX_MSGSIZE)); final int maxAttempts = 1000; int attempts = 0; @@ -105,7 +102,7 @@ public class ServerTest { this.clientDispatcher = new DispatcherImpl(new MessageFactory()); final SimpleSessionListener listener = new SimpleSessionListener(); - final Future session = this.clientDispatcher.createClient(new ProtocolConnection() { + final ProtocolSession session = this.clientDispatcher.createClient(new ProtocolConnection() { @Override public SessionPreferencesChecker getProposalChecker() { return new SimpleSessionProposalChecker(); @@ -118,7 +115,7 @@ public class ServerTest { @Override public InetSocketAddress getPeerAddress() { - return new InetSocketAddress("127.0.0.5", PORT); + return ServerTest.this.serverAddress; } @Override @@ -126,13 +123,8 @@ public class ServerTest { return listener; } }, new SimpleSessionFactory(MAX_MSGSIZE)); - try { - session.get(); - fail("Exception should have occurred."); - } catch (final ExecutionException e) { + if (session == null) listener.failed = true; - assertTrue(e.getCause() instanceof ConnectException); - } final int maxAttempts = 100; int attempts = 0; synchronized (listener) { @@ -145,11 +137,10 @@ public class ServerTest { @After public void tearDown() throws IOException { - this.dispatcher.onSessionClosed(this.session); if (this.server != null) this.server.close(); - // this.dispatcher.stop(); - // this.clientDispatcher.stop(); + this.dispatcher.close(); + this.clientDispatcher.close(); try { Thread.sleep(100); } catch (final InterruptedException e) { diff --git a/framework/src/test/java/org/opendaylight/protocol/framework/Session.java b/framework/src/test/java/org/opendaylight/protocol/framework/Session.java index bca23c3160..5108d0be1f 100644 --- a/framework/src/test/java/org/opendaylight/protocol/framework/Session.java +++ b/framework/src/test/java/org/opendaylight/protocol/framework/Session.java @@ -67,11 +67,6 @@ public class Session implements ProtocolSession { return null; } - @Override - public void onConnectionFailed(final IOException e) { - logger.debug("Connection failed: {}", e.getMessage(), e); - } - @Override public int maximumMessageSize() { return this.maxMsgSize; diff --git a/framework/src/test/java/org/opendaylight/protocol/framework/SimpleSession.java b/framework/src/test/java/org/opendaylight/protocol/framework/SimpleSession.java index 7ecb0f4591..1361e6a34e 100644 --- a/framework/src/test/java/org/opendaylight/protocol/framework/SimpleSession.java +++ b/framework/src/test/java/org/opendaylight/protocol/framework/SimpleSession.java @@ -54,11 +54,6 @@ public final class SimpleSession implements ProtocolSession { return null; } - @Override - public void onConnectionFailed(final IOException e) { - ((SimpleSessionListener) this.listener).onConnectionFailed(this, e); - } - @Override public int maximumMessageSize() { return this.maxMsgSize; diff --git a/framework/src/test/resources/logback-test.xml b/framework/src/test/resources/logback-test.xml index 2838411682..7ace93024a 100644 --- a/framework/src/test/resources/logback-test.xml +++ b/framework/src/test/resources/logback-test.xml @@ -7,7 +7,7 @@ - + diff --git a/pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSession.java b/pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSession.java index df7573c124..32d5588438 100644 --- a/pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSession.java +++ b/pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSession.java @@ -9,22 +9,24 @@ package org.opendaylight.protocol.pcep; import java.io.Closeable; +import org.opendaylight.protocol.pcep.object.PCEPCloseObject; + /** - * PCEP Session represents the finite state machine in PCEP, - * including timers and its purpose is to create a PCEP connection - * between PCE/PCC. Session is automatically started, when TCP - * connection is created, but can be stopped manually. - * If the session is up, it has to redirect messages to/from user. - * Handles also malformed messages and unknown requests. + * PCEP Session represents the finite state machine in PCEP, including timers and its purpose is to create a PCEP + * connection between PCE/PCC. Session is automatically started, when TCP connection is created, but can be stopped + * manually. If the session is up, it has to redirect messages to/from user. Handles also malformed messages and unknown + * requests. */ public interface PCEPSession extends Closeable { /** - * Sends message from user to PCE/PCC. If the user sends an Open - * Message, the session returns an error (open message is only - * allowed, when a PCEP handshake is in progress). Close message - * will close the session and free all the resources. + * Sends message from user to PCE/PCC. If the user sends an Open Message, the session returns an error (open message + * is only allowed, when a PCEP handshake is in progress). Close message will close the session and free all the + * resources. + * * @param message message to be sent */ public void sendMessage(PCEPMessage message); + + public void close(PCEPCloseObject.Reason reason); } diff --git a/pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSessionListener.java b/pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSessionListener.java index cc568f3913..a0506099f1 100644 --- a/pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSessionListener.java +++ b/pcep/api/src/main/java/org/opendaylight/protocol/pcep/PCEPSessionListener.java @@ -9,16 +9,16 @@ package org.opendaylight.protocol.pcep; import org.opendaylight.protocol.framework.SessionListener; import org.opendaylight.protocol.framework.TerminationReason; -import org.opendaylight.protocol.pcep.object.PCEPCloseObject; import org.opendaylight.protocol.pcep.object.PCEPOpenObject; /** - * Listener that receives session informations from the session. + * Listener that receives session informations from the session. */ public abstract class PCEPSessionListener implements SessionListener { /** * Fired when a message is received. + * * @param session session which received the message * @param message PCEPMessage */ @@ -26,7 +26,7 @@ public abstract class PCEPSessionListener implements SessionListener { /** * Fired when the session is in state UP. - * + * * @param session Session which went up * @param local Local open proposal which the peer accepted * @param remote Peer open proposal which we accepted @@ -34,23 +34,19 @@ public abstract class PCEPSessionListener implements SessionListener { public abstract void onSessionUp(PCEPSession session, PCEPOpenObject local, PCEPOpenObject remote); /** - * Fired when the session went down as a result of peer's decision - * to tear it down. - * Implementation should take care of closing underlying session. - * + * Fired when the session went down as a result of peer's decision to tear it down. Implementation should take care + * of closing underlying session. + * * @param session Session which went down - * @param reason Reason for termination, may be null when the underlying - * channel was closed without a specific reason. + * @param cause Reason for termination * @param e exception that caused session down */ - public abstract void onSessionDown(PCEPSession session, PCEPCloseObject reason, Exception e); + public abstract void onSessionDown(PCEPSession session, TerminationReason cause, Exception e); /** - * Fired when the session is terminated locally. The session has already - * been closed and transitioned to IDLE state. Any outstanding queued - * messages were not sent. The user should not attempt to make any use - * of the session. - * + * Fired when the session is terminated locally. The session has already been closed and transitioned to IDLE state. + * Any outstanding queued messages were not sent. The user should not attempt to make any use of the session. + * * @param session Session which went down * @param cause the cause why the session went down */ diff --git a/pcep/impl/pom.xml b/pcep/impl/pom.xml index f8178f9848..965eccc0d6 100644 --- a/pcep/impl/pom.xml +++ b/pcep/impl/pom.xml @@ -39,11 +39,6 @@ concepts ${project.version} - - com.google.guava - guava - ${guava.version} - org.slf4j slf4j-api diff --git a/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPDispatcherImpl.java b/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPDispatcherImpl.java index f8dc6ff920..715e153bae 100644 --- a/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPDispatcherImpl.java +++ b/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPDispatcherImpl.java @@ -60,13 +60,7 @@ public class PCEPDispatcherImpl implements PCEPDispatcher { */ @Override public PCEPSession createClient(final PCEPConnection connection) throws IOException { - PCEPSession session = null; - try { - session = (PCEPSession) this.dispatcher.createClient(connection, new PCEPSessionFactoryImpl(this.maxUnknownMessages)).get(); - } catch (InterruptedException | ExecutionException e) { - logger.warn("Client not created. Exception {}.", e.getMessage(), e); - } - return session; + return (PCEPSession) this.dispatcher.createClient(connection, new PCEPSessionFactoryImpl(this.maxUnknownMessages)); } @Override diff --git a/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPMessageFactory.java b/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPMessageFactory.java index b4f38ccd72..0753ebea23 100644 --- a/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPMessageFactory.java +++ b/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPMessageFactory.java @@ -132,7 +132,7 @@ public class PCEPMessageFactory implements ProtocolMessageFactory { @Override public ProtocolMessage parse(final byte[] bytes) throws DeserializerException, DocumentedException { - if (bytes == null) + if (bytes == null || bytes.length == 0) throw new IllegalArgumentException("Array of bytes is mandatory."); logger.trace("Attempt to parse message from bytes: {}", ByteArray.bytesToHexString(bytes)); diff --git a/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionImpl.java b/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionImpl.java index 8b0e3bbd99..74152e6897 100644 --- a/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionImpl.java +++ b/pcep/impl/src/main/java/org/opendaylight/protocol/pcep/impl/PCEPSessionImpl.java @@ -373,45 +373,42 @@ class PCEPSessionImpl implements PCEPSession, ProtocolSession, PCEPSessionRuntim } } - private void commonClose() { - this.changeState(State.IDLE); - this.parent.onSessionClosed(this); - } - /** - * Closes PCEP session from the parent with given reason. A message needs to be sent, but parent doesn't have to be - * modified, because he initiated the closing. (To prevent concurrent modification exception). + * Closes PCEP session without sending a Close message, as the channel is no longer active. Notify parent about + * this. * - * @param closeObject + * @param reason reason, why it was terminated */ - void closeWithoutMessage() { - logger.debug("Closing session: {}", this); - commonClose(); + @Override + public void close() { + logger.trace("Closing session: {}", this); + this.changeState(State.IDLE); + this.parent.onSessionClosed(this); } /** - * Closes PCEP session, cancels all timers, returns to state Idle WITHOUT sending the Close Message. KeepAlive and - * DeadTimer are cancelled if the state of the session changes to IDLE. This method is used to close the PCEP - * session from inside the session or from the listener, therefore the parent of this session should be informed. - * The only closing reason is UNKNOWN. + * Closes PCEP session, cancels all timers, returns to state Idle, sends the Close Message. KeepAlive and DeadTimer + * are cancelled if the state of the session changes to IDLE. This method is used to close the PCEP session from + * inside the session or from the listener, therefore the parent of this session should be informed. */ @Override - public synchronized void close() { + public synchronized void close(final PCEPCloseObject.Reason reason) { logger.debug("Closing session: {}", this); - this.sendMessage(new PCEPCloseMessage(new PCEPCloseObject(Reason.UNKNOWN))); - commonClose(); + this.sendMessage(new PCEPCloseMessage(new PCEPCloseObject(reason))); + this.changeState(State.IDLE); + this.parent.onSessionClosed(this); } private void terminate(final PCEPCloseObject.Reason reason) { - this.sendMessage(new PCEPCloseMessage(new PCEPCloseObject(reason))); - this.closeWithoutMessage(); this.listener.onSessionTerminated(this, new PCEPCloseTermination(reason)); + this.sendMessage(new PCEPCloseMessage(new PCEPCloseObject(reason))); + this.close(); } private void terminate(final PCEPErrors error) { - this.sendErrorMessage(error); - this.closeWithoutMessage(); this.listener.onSessionTerminated(this, new PCEPErrorTermination(error)); + this.sendErrorMessage(error); + this.close(); } @Override @@ -666,8 +663,7 @@ class PCEPSessionImpl implements PCEPSession, ProtocolSession, PCEPSessionRuntim * session DOWN event. */ if (pcepMsg instanceof PCEPCloseMessage) { - this.listener.onSessionTerminated(this, new PCEPCloseTermination(((PCEPCloseMessage) pcepMsg).getCloseObject().getReason())); - this.closeWithoutMessage(); + this.close(); return; } this.listener.onMessage(this, pcepMsg); @@ -678,12 +674,6 @@ class PCEPSessionImpl implements PCEPSession, ProtocolSession, PCEPSessionRuntim return this.factory; } - @Override - public void onConnectionFailed(final IOException e) { - logger.info("Connection failed before finishing: {}", e.getMessage(), e); - this.listener.onSessionDown(this, new PCEPCloseObject(Reason.UNKNOWN), e); - } - /** * @return the sentMsgCount */ diff --git a/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/MockPCE.java b/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/MockPCE.java index 7d9278c03f..34ce86077d 100644 --- a/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/MockPCE.java +++ b/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/MockPCE.java @@ -10,18 +10,16 @@ package org.opendaylight.protocol.pcep.impl; import java.util.ArrayList; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.opendaylight.protocol.framework.TerminationReason; import org.opendaylight.protocol.pcep.PCEPErrors; import org.opendaylight.protocol.pcep.PCEPMessage; import org.opendaylight.protocol.pcep.PCEPSession; import org.opendaylight.protocol.pcep.PCEPSessionListener; import org.opendaylight.protocol.pcep.message.PCEPErrorMessage; -import org.opendaylight.protocol.pcep.object.PCEPCloseObject; import org.opendaylight.protocol.pcep.object.PCEPErrorObject; import org.opendaylight.protocol.pcep.object.PCEPOpenObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * @@ -41,12 +39,11 @@ public class MockPCE extends PCEPSessionListener { public MockPCE() { } - public void sendMessage(PCEPMessage msg) { + public void sendMessage(final PCEPMessage msg) { this.session.handleMessage(msg); } - public void sendErrorMessage(PCEPErrors value, - PCEPOpenObject open) { + public void sendErrorMessage(final PCEPErrors value, final PCEPOpenObject open) { final PCEPErrorObject error = new PCEPErrorObject(value); final List errors = new ArrayList(); errors.add(error); @@ -57,34 +54,32 @@ public class MockPCE extends PCEPSessionListener { return this.listMsg; } - public void addSession(PCEPSessionImpl l) { + public void addSession(final PCEPSessionImpl l) { this.session = l; } @Override - public void onMessage(PCEPSession session, PCEPMessage message) { + public void onMessage(final PCEPSession session, final PCEPMessage message) { this.listMsg.add(message); - logger.debug("Message received:" + message); + logger.debug("Message received: {}", message); } @Override - public void onSessionUp(PCEPSession session, PCEPOpenObject local, - PCEPOpenObject remote) { + public void onSessionUp(final PCEPSession session, final PCEPOpenObject local, final PCEPOpenObject remote) { logger.debug("Session Up"); this.up = true; this.notifyAll(); } @Override - public void onSessionDown(PCEPSession session, PCEPCloseObject reason, Exception e) { - logger.debug("Session Down"); + public void onSessionDown(final PCEPSession session, final TerminationReason reason, final Exception e) { + logger.debug("Session Down. Cause {} or {}.", reason, e); this.down = true; - //this.notifyAll(); + // this.notifyAll(); } @Override - public void onSessionTerminated(PCEPSession session, - TerminationReason cause) { - logger.debug("Session terminated. Cause : " + cause.toString()); + public void onSessionTerminated(final PCEPSession session, final TerminationReason cause) { + logger.debug("Session terminated. Cause : {}", cause); } } diff --git a/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/SimpleSessionListener.java b/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/SimpleSessionListener.java index 3f7b5a7d7b..9b145d1809 100644 --- a/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/SimpleSessionListener.java +++ b/pcep/impl/src/test/java/org/opendaylight/protocol/pcep/impl/SimpleSessionListener.java @@ -10,15 +10,13 @@ package org.opendaylight.protocol.pcep.impl; import java.util.ArrayList; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.opendaylight.protocol.framework.TerminationReason; import org.opendaylight.protocol.pcep.PCEPMessage; import org.opendaylight.protocol.pcep.PCEPSession; import org.opendaylight.protocol.pcep.PCEPSessionListener; -import org.opendaylight.protocol.pcep.object.PCEPCloseObject; import org.opendaylight.protocol.pcep.object.PCEPOpenObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Simple Session Listener that is notified about messages and changes in the session. @@ -35,29 +33,27 @@ public class SimpleSessionListener extends PCEPSessionListener { } @Override - public void onMessage(PCEPSession session, PCEPMessage message) { - logger.debug("Received message: " + message.getClass() + " " + message); + public void onMessage(final PCEPSession session, final PCEPMessage message) { + logger.debug("Received message: {} {}", message.getClass(), message); this.messages.add(message); } @Override - public synchronized void onSessionUp(PCEPSession session, PCEPOpenObject local, - PCEPOpenObject remote) { + public synchronized void onSessionUp(final PCEPSession session, final PCEPOpenObject local, final PCEPOpenObject remote) { logger.debug("Session up."); this.up = true; this.notifyAll(); } @Override - public void onSessionDown(PCEPSession session, PCEPCloseObject reason, Exception e) { - logger.debug("Session down."); + public void onSessionDown(final PCEPSession session, final TerminationReason reason, final Exception e) { + logger.debug("Session down. Cause: {} or {}", reason, e); this.up = false; - //this.notifyAll(); + // this.notifyAll(); } @Override - public void onSessionTerminated(PCEPSession session, - TerminationReason cause) { - logger.debug("Session terminated. Cause : " + cause.toString()); + public void onSessionTerminated(final PCEPSession session, final TerminationReason cause) { + logger.debug("Session terminated. Cause : ", cause.toString()); } } diff --git a/pcep/testtool/pom.xml b/pcep/testtool/pom.xml index 6a33930677..ac226dacff 100644 --- a/pcep/testtool/pom.xml +++ b/pcep/testtool/pom.xml @@ -44,11 +44,6 @@ slf4j-api ${slf4j.version} - - org.codehaus.groovy - groovy-all - 2.0.2 - ch.qos.logback logback-classic diff --git a/pcep/testtool/src/main/java/org/opendaylight/protocol/pcep/testtool/SimpleSessionListener.java b/pcep/testtool/src/main/java/org/opendaylight/protocol/pcep/testtool/SimpleSessionListener.java index fb9fdb5825..c9c5cc3e8d 100644 --- a/pcep/testtool/src/main/java/org/opendaylight/protocol/pcep/testtool/SimpleSessionListener.java +++ b/pcep/testtool/src/main/java/org/opendaylight/protocol/pcep/testtool/SimpleSessionListener.java @@ -10,15 +10,13 @@ package org.opendaylight.protocol.pcep.testtool; import java.util.ArrayList; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.opendaylight.protocol.framework.TerminationReason; import org.opendaylight.protocol.pcep.PCEPMessage; import org.opendaylight.protocol.pcep.PCEPSession; import org.opendaylight.protocol.pcep.PCEPSessionListener; -import org.opendaylight.protocol.pcep.object.PCEPCloseObject; import org.opendaylight.protocol.pcep.object.PCEPOpenObject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Simple Session Listener that is notified about messages and changes in the session. @@ -35,29 +33,27 @@ public class SimpleSessionListener extends PCEPSessionListener { } @Override - public void onMessage(PCEPSession session, PCEPMessage message) { - logger.debug("Received message: " + message); + public void onMessage(final PCEPSession session, final PCEPMessage message) { + logger.debug("Received message: {}", message); this.messages.add(message); } @Override - public void onSessionUp(PCEPSession session, PCEPOpenObject local, - PCEPOpenObject remote) { + public void onSessionUp(final PCEPSession session, final PCEPOpenObject local, final PCEPOpenObject remote) { logger.debug("Session up."); this.up = true; - //this.notifyAll(); + // this.notifyAll(); } @Override - public void onSessionDown(PCEPSession session, PCEPCloseObject reason, Exception e) { - logger.debug("Session down."); + public void onSessionDown(final PCEPSession session, final TerminationReason reason, final Exception e) { + logger.debug("Session down. Cause : {} or {}", reason, e); this.up = false; - //this.notifyAll(); + // this.notifyAll(); } @Override - public void onSessionTerminated(PCEPSession session, - TerminationReason cause) { - logger.debug("Session terminated. Cause : " + cause.toString()); + public void onSessionTerminated(final PCEPSession session, final TerminationReason cause) { + logger.debug("Session terminated. Cause : {}", cause); } } diff --git a/pcep/testtool/src/main/java/org/opendaylight/protocol/pcep/testtool/TestingSessionListener.java b/pcep/testtool/src/main/java/org/opendaylight/protocol/pcep/testtool/TestingSessionListener.java index c259eee9cf..8befe95711 100644 --- a/pcep/testtool/src/main/java/org/opendaylight/protocol/pcep/testtool/TestingSessionListener.java +++ b/pcep/testtool/src/main/java/org/opendaylight/protocol/pcep/testtool/TestingSessionListener.java @@ -7,6 +7,7 @@ */ package org.opendaylight.protocol.pcep.testtool; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -18,7 +19,6 @@ import org.opendaylight.protocol.pcep.PCEPMessage; import org.opendaylight.protocol.pcep.PCEPSession; import org.opendaylight.protocol.pcep.PCEPSessionListener; import org.opendaylight.protocol.pcep.message.PCEPXRAddTunnelMessage; -import org.opendaylight.protocol.pcep.object.PCEPCloseObject; import org.opendaylight.protocol.pcep.object.PCEPEndPointsObject; import org.opendaylight.protocol.pcep.object.PCEPExplicitRouteObject; import org.opendaylight.protocol.pcep.object.PCEPLspObject; @@ -54,8 +54,13 @@ public class TestingSessionListener extends PCEPSessionListener { } @Override - public void onSessionDown(final PCEPSession session, final PCEPCloseObject reason, final Exception e) { - logger.debug("Session down because: {}", reason); + public void onSessionDown(final PCEPSession session, final TerminationReason cause, final Exception e) { + logger.debug("Session down with cause : {} or exception: {}", cause, e); + try { + session.close(); + } catch (final IOException e1) { + logger.debug("Could not close session, because {}", e1.getMessage(), e1); + } } @Override -- 2.36.6