From 07c95aeff6efbc072e0c4ea40881cecdd10067d8 Mon Sep 17 00:00:00 2001 From: Martin Bobak Date: Wed, 17 Dec 2014 09:22:42 +0100 Subject: [PATCH] Bug 2525 : remove zombie registrations of notification listener Change-Id: I2863fa74cd7b287d760cf55e190dbceb4df629f4 Signed-off-by: Martin Bobak --- .../md/core/sal/SalRegistrationManager.java | 23 ++++++------ .../core/sal/SalRegistrationManagerTest.java | 37 +++++++++---------- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/SalRegistrationManager.java b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/SalRegistrationManager.java index ea954019fa..1e8d3d67da 100644 --- a/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/SalRegistrationManager.java +++ b/openflowplugin/src/main/java/org/opendaylight/openflowplugin/openflow/md/core/sal/SalRegistrationManager.java @@ -7,12 +7,12 @@ */ package org.opendaylight.openflowplugin.openflow.md.core.sal; +import com.google.common.base.Preconditions; import java.math.BigInteger; import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; - import org.opendaylight.controller.md.sal.binding.api.DataBroker; import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.ProviderContext; import org.opendaylight.controller.sal.binding.api.NotificationProviderService; @@ -45,8 +45,6 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.InstanceIdenti import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Preconditions; - /** * session and inventory listener implementation */ @@ -106,7 +104,7 @@ public class SalRegistrationManager implements SessionListener, AutoCloseable { LOG.debug("ModelDrivenSwitch for {} registered to MD-SAL.", datapathId.toString()); NotificationQueueWrapper wrappedNotification = new NotificationQueueWrapper( - nodeAdded(ofSwitch, features, nodeRef), + nodeAdded(ofSwitch, features, nodeRef), context.getFeatures().getVersion()); context.getNotificationEnqueuer().enqueueNotification(wrappedNotification); } @@ -118,13 +116,14 @@ public class SalRegistrationManager implements SessionListener, AutoCloseable { InstanceIdentifier identifier = identifierFromDatapathId(datapathId); NodeRef nodeRef = new NodeRef(identifier); NodeRemoved nodeRemoved = nodeRemoved(nodeRef); - if (context.isValid()) { - CompositeObjectRegistration registration = context.getProviderRegistration(); + + CompositeObjectRegistration registration = context.getProviderRegistration(); + if (null != registration) { registration.close(); + context.setProviderRegistration(null); } - LOG.debug("ModelDrivenSwitch for {} unregistered from MD-SAL.", datapathId.toString()); - + NotificationQueueWrapper wrappedNotification = new NotificationQueueWrapper( nodeRemoved, context.getFeatures().getVersion()); context.getNotificationEnqueuer().enqueueNotification(wrappedNotification); @@ -153,10 +152,10 @@ public class SalRegistrationManager implements SessionListener, AutoCloseable { // LOG.warn("IP address of the node {} cannot be obtained. Session is not valid.", sw.getNodeId()); // return null; // } - Preconditions.checkNotNull(sessionContext.getPrimaryConductor(), - "primary conductor must not be NULL -> "+sw.getNodeId()); + Preconditions.checkNotNull(sessionContext.getPrimaryConductor(), + "primary conductor must not be NULL -> " + sw.getNodeId()); Preconditions.checkNotNull(sessionContext.getPrimaryConductor().getConnectionAdapter(), - "connection adapter of primary conductor must not be NULL -> "+sw.getNodeId()); + "connection adapter of primary conductor must not be NULL -> " + sw.getNodeId()); InetSocketAddress remoteAddress = sessionContext.getPrimaryConductor().getConnectionAdapter() .getRemoteAddress(); if (remoteAddress == null) { @@ -213,7 +212,7 @@ public class SalRegistrationManager implements SessionListener, AutoCloseable { sessionListenerRegistration.close(); } } - + /** * @param providerContext the providerContext to set */ diff --git a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/SalRegistrationManagerTest.java b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/SalRegistrationManagerTest.java index 9bc8fd9bac..2047cb5047 100644 --- a/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/SalRegistrationManagerTest.java +++ b/openflowplugin/src/test/java/org/opendaylight/openflowplugin/openflow/md/core/sal/SalRegistrationManagerTest.java @@ -10,15 +10,13 @@ package org.opendaylight.openflowplugin.openflow.md.core.sal; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListeningExecutorService; - import java.math.BigInteger; import java.util.Collections; import java.util.Set; -import java.util.concurrent.ExecutorService; - import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -28,18 +26,17 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import org.opendaylight.controller.sal.binding.api.BindingAwareBroker; -import org.opendaylight.controller.sal.binding.api.NotificationListener; import org.opendaylight.controller.sal.binding.api.NotificationProviderService; import org.opendaylight.controller.sal.common.util.Rpcs; -import org.opendaylight.openflowplugin.api.openflow.md.ModelDrivenSwitch; import org.opendaylight.openflowplugin.api.OFConstants; +import org.opendaylight.openflowplugin.api.openflow.md.ModelDrivenSwitch; import org.opendaylight.openflowplugin.api.openflow.md.core.ConnectionConductor; import org.opendaylight.openflowplugin.api.openflow.md.core.NotificationEnqueuer; import org.opendaylight.openflowplugin.api.openflow.md.core.SwitchConnectionDistinguisher; import org.opendaylight.openflowplugin.api.openflow.md.core.session.IMessageDispatchService; -import org.opendaylight.openflowplugin.openflow.md.core.session.OFSessionUtil; -import org.opendaylight.openflowplugin.api.openflow.md.core.session.SessionContext; import org.opendaylight.openflowplugin.api.openflow.md.core.session.SwitchSessionKeyOF; +import org.opendaylight.openflowplugin.openflow.md.core.session.OFSessionUtil; +import org.opendaylight.openflowplugin.openflow.md.core.session.SessionContextOFImpl; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.UpdateFlowOutput; import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.UpdateFlowOutputBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId; @@ -48,10 +45,8 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.N import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.FlowModInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.openflow.protocol.rev130731.GetFeaturesOutput; import org.opendaylight.yangtools.concepts.CompositeObjectRegistration; -import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.binding.KeyedInstanceIdentifier; -import org.opendaylight.yangtools.yang.binding.Notification; import org.opendaylight.yangtools.yang.common.RpcError; import org.opendaylight.yangtools.yang.common.RpcResult; @@ -65,8 +60,8 @@ public class SalRegistrationManagerTest { private static final BigInteger dataPathId = BigInteger.ONE; private SalRegistrationManager salRegistrationManager; - @Mock - private SessionContext context; + + private SessionContextOFImpl context; @Mock private ConnectionConductor conductor; @Mock @@ -90,20 +85,19 @@ public class SalRegistrationManagerTest { @Before public void setUp() { OFSessionUtil.getSessionManager().setRpcPool(rpcPool); - - Mockito.when(context.getPrimaryConductor()).thenReturn(conductor); - Mockito.when(context.getMessageDispatchService()).thenReturn(messageDispatchService); Mockito.when(conductor.getVersion()).thenReturn(OFConstants.OFP_VERSION_1_0) .thenReturn(OFConstants.OFP_VERSION_1_3); - Mockito.when(context.getFeatures()).thenReturn(features); + context = new SessionContextOFImpl(); + context.setPrimaryConductor(conductor); + Mockito.when(features.getDatapathId()).thenReturn(BigInteger.valueOf(1)); + Mockito.when(features.getVersion()).thenReturn((short) 1); + context.setFeatures(features); + context.setNotificationEnqueuer(notificationEnqueuer); mdSwitchOF13 = new ModelDrivenSwitchImpl(null, null, context); registration = new CompositeObjectRegistration<>(mdSwitchOF13, Collections.EMPTY_LIST); + context.setProviderRegistration(registration); - Mockito.when(context.getProviderRegistration()).thenReturn(registration); - Mockito.when(context.getNotificationEnqueuer()).thenReturn(notificationEnqueuer); - Mockito.when(features.getDatapathId()).thenReturn(BigInteger.valueOf(1)); - Mockito.when(features.getVersion()).thenReturn((short) 1); Set errorSet = Collections.emptySet(); UpdateFlowOutputBuilder updateFlowOutput = new UpdateFlowOutputBuilder(); @@ -116,8 +110,9 @@ public class SalRegistrationManagerTest { salRegistrationManager = new SalRegistrationManager(); salRegistrationManager.onSessionInitiated(providerContext); salRegistrationManager.setPublishService(notificationProviderService); + } - + /** * free sesion manager */ @@ -170,7 +165,9 @@ public class SalRegistrationManagerTest { */ @Test public void testOnSessionRemoved() { + assertNotNull(context.getProviderRegistration()); salRegistrationManager.onSessionRemoved(context); + assertNull(context.getProviderRegistration()); } /** -- 2.36.6