From 4d94935b354ae96badf10d8f70dd293a34f8c2d2 Mon Sep 17 00:00:00 2001 From: Gilles Thouenon Date: Fri, 17 Mar 2023 12:23:09 +0100 Subject: [PATCH] Unregister DataTreeChangeListeners - use a list of listeners of DataTreeChangeListeners when serveral are registered. - unregister all listeners when the bundle stop. - add a check in few provider UT Signed-off-by: Gilles Thouenon Change-Id: Ib564b7b74e2b499c129c4c76c0e5a38012853dd7 (cherry picked from commit 3f8fa9fb8da822fbc6579bbaaf49a95b499e9a27) --- .../inventory/ListenerProvider.java | 30 ++++++++----- .../networkmodel/NetworkModelProvider.java | 22 +++++----- .../NetworkModelProviderTest.java | 12 +++--- .../impl/ServicehandlerProviderTest.java | 12 ++++-- .../transportpce/tapi/impl/TapiProvider.java | 42 ++++++++----------- .../tapi/provider/TapiProviderTest.java | 30 +++++++------ 6 files changed, 80 insertions(+), 68 deletions(-) diff --git a/inventory/src/main/java/org/opendaylight/transportpce/inventory/ListenerProvider.java b/inventory/src/main/java/org/opendaylight/transportpce/inventory/ListenerProvider.java index 7f6578b1b..651621aa5 100644 --- a/inventory/src/main/java/org/opendaylight/transportpce/inventory/ListenerProvider.java +++ b/inventory/src/main/java/org/opendaylight/transportpce/inventory/ListenerProvider.java @@ -7,6 +7,8 @@ */ package org.opendaylight.transportpce.inventory; +import java.util.ArrayList; +import java.util.List; import javax.sql.DataSource; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.DataTreeChangeListener; @@ -20,8 +22,10 @@ import org.opendaylight.transportpce.inventory.listener.DeviceListener; import org.opendaylight.transportpce.inventory.listener.OverlayNetworkChangeListener; import org.opendaylight.transportpce.inventory.listener.UnderlayNetworkChangeListener; import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node; +import org.opendaylight.yangtools.concepts.Registration; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Deactivate; import org.osgi.service.component.annotations.Reference; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,6 +37,7 @@ import org.slf4j.LoggerFactory; public class ListenerProvider { private static final Logger LOG = LoggerFactory.getLogger(ListenerProvider.class); + private List listeners = new ArrayList<>(); /** * Constructor invoked by blueprint injects all dependencies. @@ -48,33 +53,38 @@ public class ListenerProvider { LOG.debug("Registering listeners..."); OverlayNetworkChangeListener overlayNetworkListener = new OverlayNetworkChangeListener(); - dataBroker.registerDataTreeChangeListener( + listeners.add(dataBroker.registerDataTreeChangeListener( DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, InstanceIdentifiers.OVERLAY_NETWORK_II), - overlayNetworkListener); + overlayNetworkListener)); LOG.info("Overlay network change listener was successfully registered"); UnderlayNetworkChangeListener underlayNetworkListener = new UnderlayNetworkChangeListener(); - dataBroker.registerDataTreeChangeListener( + listeners.add(dataBroker.registerDataTreeChangeListener( DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, InstanceIdentifiers.UNDERLAY_NETWORK_II), - underlayNetworkListener); + underlayNetworkListener)); LOG.info("Underlay network change listener was successfully registered"); ClliNetworkChangeListener clliNetworkChangeListener = new ClliNetworkChangeListener(); - dataBroker.registerDataTreeChangeListener( + listeners.add(dataBroker.registerDataTreeChangeListener( DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, InstanceIdentifiers.CLLI_NETWORK_II), - clliNetworkChangeListener); + clliNetworkChangeListener)); LOG.info("CLLI network change listener was successfully registered"); INode121 inode121 = new INode121(dataSource, deviceTransactionManager); INode inode = new INode(dataSource, inode121); DeviceInventory deviceInventory = new DeviceInventory(dataSource, inode); DeviceListener deviceListener = new DeviceListener(deviceInventory); - dataBroker.registerDataTreeChangeListener( + listeners.add(dataBroker.registerDataTreeChangeListener( DataTreeIdentifier.create(LogicalDatastoreType.OPERATIONAL, - InstanceIdentifiers.NETCONF_TOPOLOGY_II.child(Node.class)), deviceListener); + InstanceIdentifiers.NETCONF_TOPOLOGY_II.child(Node.class)), deviceListener)); LOG.info("Device change listener was successfully registered"); DeviceConfigListener deviceConfigListener = new DeviceConfigListener(deviceInventory); - dataBroker.registerDataTreeChangeListener( + listeners.add(dataBroker.registerDataTreeChangeListener( DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, - InstanceIdentifiers.NETCONF_TOPOLOGY_II.child(Node.class)), deviceConfigListener); + InstanceIdentifiers.NETCONF_TOPOLOGY_II.child(Node.class)), deviceConfigListener)); LOG.info("Device config change listener was successfully registered"); } + @Deactivate + public void close() { + listeners.forEach(lis -> lis.close()); + listeners.clear(); + } } diff --git a/networkmodel/src/main/java/org/opendaylight/transportpce/networkmodel/NetworkModelProvider.java b/networkmodel/src/main/java/org/opendaylight/transportpce/networkmodel/NetworkModelProvider.java index 571f9076c..a1c359452 100644 --- a/networkmodel/src/main/java/org/opendaylight/transportpce/networkmodel/NetworkModelProvider.java +++ b/networkmodel/src/main/java/org/opendaylight/transportpce/networkmodel/NetworkModelProvider.java @@ -7,6 +7,8 @@ */ package org.opendaylight.transportpce.networkmodel; +import java.util.ArrayList; +import java.util.List; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.DataTreeIdentifier; @@ -51,8 +53,7 @@ public class NetworkModelProvider { private final RpcProviderService rpcProviderService; private final TransportpceNetworkutilsService networkutilsService; private final NetConfTopologyListener topologyListener; - private ListenerRegistration dataTreeChangeListenerRegistration; - private ListenerRegistration mappingListenerRegistration; + private List listeners; private @NonNull Registration networkutilsServiceRpcRegistration; private TpceNetwork tpceNetwork; private ListenerRegistration serviceHandlerListenerRegistration; @@ -73,6 +74,7 @@ public class NetworkModelProvider { this.rpcProviderService = rpcProviderService; this.notificationService = notificationService; this.frequenciesService = frequenciesService; + this.listeners = new ArrayList<>(); this.networkutilsService = new NetworkUtilsImpl(dataBroker); this.topologyListener = new NetConfTopologyListener(networkModelService, dataBroker, deviceTransactionManager, portMapping); @@ -90,11 +92,11 @@ public class NetworkModelProvider { tpceNetwork.createLayer(NetworkUtils.UNDERLAY_NETWORK_ID); tpceNetwork.createLayer(NetworkUtils.OVERLAY_NETWORK_ID); tpceNetwork.createLayer(NetworkUtils.OTN_NETWORK_ID); - dataTreeChangeListenerRegistration = dataBroker.registerDataTreeChangeListener( + listeners.add(dataBroker.registerDataTreeChangeListener( DataTreeIdentifier.create(LogicalDatastoreType.OPERATIONAL, - InstanceIdentifiers.NETCONF_TOPOLOGY_II.child(Node.class)), topologyListener); - mappingListenerRegistration = dataBroker.registerDataTreeChangeListener( - DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, MAPPING_II), portMappingListener); + InstanceIdentifiers.NETCONF_TOPOLOGY_II.child(Node.class)), topologyListener)); + listeners.add(dataBroker.registerDataTreeChangeListener( + DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, MAPPING_II), portMappingListener)); networkutilsServiceRpcRegistration = rpcProviderService .registerRpcImplementation(TransportpceNetworkutilsService.class, networkutilsService); TransportpceServicehandlerListener serviceHandlerListner = new ServiceHandlerListener(frequenciesService); @@ -107,12 +109,8 @@ public class NetworkModelProvider { @Deactivate public void close() { LOG.info("NetworkModelProvider Closed"); - if (dataTreeChangeListenerRegistration != null) { - dataTreeChangeListenerRegistration.close(); - } - if (mappingListenerRegistration != null) { - mappingListenerRegistration.close(); - } + listeners.forEach(lis -> lis.close()); + listeners.clear(); if (networkutilsServiceRpcRegistration != null) { networkutilsServiceRpcRegistration.close(); } diff --git a/networkmodel/src/test/java/org/opendaylight/transportpce/networkmodel/NetworkModelProviderTest.java b/networkmodel/src/test/java/org/opendaylight/transportpce/networkmodel/NetworkModelProviderTest.java index 451e63ba5..8d5ba9f8c 100644 --- a/networkmodel/src/test/java/org/opendaylight/transportpce/networkmodel/NetworkModelProviderTest.java +++ b/networkmodel/src/test/java/org/opendaylight/transportpce/networkmodel/NetworkModelProviderTest.java @@ -19,6 +19,7 @@ import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.stubbing.Answer; +import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.NotificationService; import org.opendaylight.mdsal.binding.api.RpcProviderService; import org.opendaylight.mdsal.common.api.CommitInfo; @@ -27,14 +28,15 @@ import org.opendaylight.transportpce.common.mapping.PortMapping; import org.opendaylight.transportpce.common.network.NetworkTransactionService; import org.opendaylight.transportpce.networkmodel.service.FrequenciesService; import org.opendaylight.transportpce.networkmodel.service.NetworkModelService; -import org.opendaylight.transportpce.test.AbstractTest; import org.opendaylight.yang.gen.v1.http.org.opendaylight.transportpce.networkutils.rev220630.TransportpceNetworkutilsService; @ExtendWith(MockitoExtension.class) -public class NetworkModelProviderTest extends AbstractTest { +public class NetworkModelProviderTest { @Mock NetworkTransactionService networkTransactionService; @Mock + DataBroker dataBroker; + @Mock RpcProviderService rpcProviderService; @Mock NetworkModelService networkModelService; @@ -62,11 +64,11 @@ public class NetworkModelProviderTest extends AbstractTest { }; when(networkTransactionService.commit()).then(answer); - new NetworkModelProvider(networkTransactionService, getDataBroker(), - rpcProviderService, networkModelService, deviceTransactionManager, portMapping, notificationService, - frequenciesService); + new NetworkModelProvider(networkTransactionService, dataBroker, rpcProviderService, networkModelService, + deviceTransactionManager, portMapping, notificationService, frequenciesService); verify(rpcProviderService, times(1)) .registerRpcImplementation(any(), any(TransportpceNetworkutilsService.class)); + verify(dataBroker, times(2)).registerDataTreeChangeListener(any(), any()); } } diff --git a/servicehandler/src/test/java/org/opendaylight/transportpce/servicehandler/impl/ServicehandlerProviderTest.java b/servicehandler/src/test/java/org/opendaylight/transportpce/servicehandler/impl/ServicehandlerProviderTest.java index 8a696ab7e..0092b0969 100644 --- a/servicehandler/src/test/java/org/opendaylight/transportpce/servicehandler/impl/ServicehandlerProviderTest.java +++ b/servicehandler/src/test/java/org/opendaylight/transportpce/servicehandler/impl/ServicehandlerProviderTest.java @@ -15,6 +15,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.DataTreeChangeListener; import org.opendaylight.mdsal.binding.api.NotificationPublishService; import org.opendaylight.mdsal.binding.api.RpcProviderService; @@ -27,8 +28,10 @@ import org.opendaylight.yang.gen.v1.http.org.openroadm.service.rev211210.OrgOpen import org.opendaylight.yang.gen.v1.http.org.openroadm.service.rev211210.service.list.Services; @ExtendWith(MockitoExtension.class) -public class ServicehandlerProviderTest extends AbstractTest { +public class ServicehandlerProviderTest extends AbstractTest { + @Mock + DataBroker dataBroker; @Mock RpcProviderService rpcProviderRegistry; @Mock @@ -48,10 +51,11 @@ public class ServicehandlerProviderTest extends AbstractTest { @Test void testInitRegisterServiceHandlerToRpcRegistry() { - ServicehandlerProvider provider = new ServicehandlerProvider(getDataBroker(), rpcProviderRegistry, - getNotificationService() , serviceDataStoreOperations, pceListenerImpl, rendererListenerImpl, - networkModelListenerImpl, notificationPublishService, servicehandler, serviceListener); + new ServicehandlerProvider(dataBroker, rpcProviderRegistry, getNotificationService() , + serviceDataStoreOperations, pceListenerImpl, rendererListenerImpl, networkModelListenerImpl, + notificationPublishService, servicehandler, serviceListener); verify(rpcProviderRegistry, times(1)).registerRpcImplementation(any(), any(OrgOpenroadmServiceService.class)); + verify(dataBroker, times(1)).registerDataTreeChangeListener(any(), any()); } } \ No newline at end of file diff --git a/tapi/src/main/java/org/opendaylight/transportpce/tapi/impl/TapiProvider.java b/tapi/src/main/java/org/opendaylight/transportpce/tapi/impl/TapiProvider.java index 8ce95c339..dc03a1f3e 100644 --- a/tapi/src/main/java/org/opendaylight/transportpce/tapi/impl/TapiProvider.java +++ b/tapi/src/main/java/org/opendaylight/transportpce/tapi/impl/TapiProvider.java @@ -7,7 +7,9 @@ */ package org.opendaylight.transportpce.tapi.impl; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.DataTreeIdentifier; import org.opendaylight.mdsal.binding.api.NotificationPublishService; @@ -54,6 +56,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.tapi.rev import org.opendaylight.yang.gen.v1.urn.tbd.params.xml.ns.yang.network.topology.rev131021.network.topology.topology.Node; import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.concepts.ObjectRegistration; +import org.opendaylight.yangtools.concepts.Registration; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; @@ -90,9 +93,7 @@ public class TapiProvider { private TapiNotificationListener tapiNetworkModelListenerImpl; private ObjectRegistration rpcRegistration; private ObjectRegistration tapiNetworkutilsServiceRpcRegistration; - private ListenerRegistration dataTreeChangeListenerRegistration; - private ListenerRegistration dataTreeChangeListenerRegistration1; - private ListenerRegistration mappingListenerListenerRegistration; + private List listeners; private ListenerRegistration pcelistenerRegistration; private ListenerRegistration rendererlistenerRegistration; private ListenerRegistration servicehandlerlistenerRegistration; @@ -141,27 +142,27 @@ public class TapiProvider { rpcProviderService.registerRpcImplementation(TapiTopologyService.class, topo); rpcProviderService.registerRpcImplementation(TapiCommonService.class, topo); + this.listeners = new ArrayList<>(); TapiNetconfTopologyListener topologyListener = new TapiNetconfTopologyListener(tapiNetworkModelServiceImpl); TapiOrLinkListener orLinkListener = new TapiOrLinkListener(tapiLink, networkTransactionService); TapiPortMappingListener tapiPortMappingListener = new TapiPortMappingListener(tapiNetworkModelServiceImpl); - dataTreeChangeListenerRegistration1 = - dataBroker.registerDataTreeChangeListener(DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, - LINK_II), orLinkListener); - dataTreeChangeListenerRegistration = - dataBroker.registerDataTreeChangeListener(DataTreeIdentifier.create(LogicalDatastoreType.OPERATIONAL, - InstanceIdentifiers.NETCONF_TOPOLOGY_II.child(Node.class)), topologyListener); - mappingListenerListenerRegistration = - dataBroker.registerDataTreeChangeListener(DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, - MAPPING_II), tapiPortMappingListener); + listeners.add(dataBroker.registerDataTreeChangeListener( + DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, LINK_II), orLinkListener)); + listeners.add(dataBroker.registerDataTreeChangeListener( + DataTreeIdentifier.create(LogicalDatastoreType.OPERATIONAL, InstanceIdentifiers.NETCONF_TOPOLOGY_II + .child(Node.class)), + topologyListener)); + listeners.add(dataBroker.registerDataTreeChangeListener( + DataTreeIdentifier.create(LogicalDatastoreType.CONFIGURATION, MAPPING_II), tapiPortMappingListener)); tapiNetworkutilsServiceRpcRegistration = rpcProviderService.registerRpcImplementation(TransportpceTapinetworkutilsService.class, this.tapiNetworkUtils); TapiListener tapiListener = new TapiListener(); - dataBroker.registerDataTreeChangeListener( + listeners.add(dataBroker.registerDataTreeChangeListener( DataTreeIdentifier.create( LogicalDatastoreType.CONFIGURATION, InstanceIdentifier.create(ServiceInterfacePoints.class)), - tapiListener); + tapiListener)); // Notification Listener pcelistenerRegistration = notificationService.registerNotificationListener(pceListenerImpl); rendererlistenerRegistration = notificationService.registerNotificationListener(rendererListenerImpl); @@ -177,16 +178,8 @@ public class TapiProvider { */ @Deactivate public void close() { - LOG.info("TapiProvider Session Closed"); - if (dataTreeChangeListenerRegistration != null) { - dataTreeChangeListenerRegistration.close(); - } - if (mappingListenerListenerRegistration != null) { - mappingListenerListenerRegistration.close(); - } - if (dataTreeChangeListenerRegistration1 != null) { - dataTreeChangeListenerRegistration1.close(); - } + listeners.forEach(lis -> lis.close()); + listeners.clear(); if (tapiNetworkutilsServiceRpcRegistration != null) { tapiNetworkutilsServiceRpcRegistration.close(); } @@ -195,5 +188,6 @@ public class TapiProvider { servicehandlerlistenerRegistration.close(); rpcRegistration.close(); tapinetworkmodellistenerRegistration.close(); + LOG.info("TapiProvider Session Closed"); } } diff --git a/tapi/src/test/java/org/opendaylight/transportpce/tapi/provider/TapiProviderTest.java b/tapi/src/test/java/org/opendaylight/transportpce/tapi/provider/TapiProviderTest.java index 9427914c4..b815d1b53 100644 --- a/tapi/src/test/java/org/opendaylight/transportpce/tapi/provider/TapiProviderTest.java +++ b/tapi/src/test/java/org/opendaylight/transportpce/tapi/provider/TapiProviderTest.java @@ -8,23 +8,26 @@ package org.opendaylight.transportpce.tapi.provider; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.opendaylight.mdsal.common.api.CommitInfo.emptyFluentFuture; -import org.junit.jupiter.api.BeforeAll; +import com.google.common.util.concurrent.Futures; +import java.util.Optional; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.opendaylight.mdsal.binding.api.DataBroker; import org.opendaylight.mdsal.binding.api.NotificationPublishService; import org.opendaylight.mdsal.binding.api.NotificationService; import org.opendaylight.mdsal.binding.api.RpcProviderService; -import org.opendaylight.transportpce.common.network.NetworkTransactionImpl; import org.opendaylight.transportpce.common.network.NetworkTransactionService; import org.opendaylight.transportpce.servicehandler.service.ServiceDataStoreOperations; import org.opendaylight.transportpce.tapi.impl.TapiProvider; import org.opendaylight.transportpce.tapi.topology.TapiNetworkModelService; -import org.opendaylight.transportpce.test.AbstractTest; import org.opendaylight.yang.gen.v1.http.org.opendaylight.transportpce.tapinetworkutils.rev210408.TransportpceTapinetworkutilsService; import org.opendaylight.yang.gen.v1.http.org.openroadm.service.rev211210.OrgOpenroadmServiceService; import org.opendaylight.yang.gen.v1.urn.onf.otcc.yang.tapi.common.rev181210.TapiCommonService; @@ -33,9 +36,10 @@ import org.opendaylight.yang.gen.v1.urn.onf.otcc.yang.tapi.notification.rev18121 import org.opendaylight.yang.gen.v1.urn.onf.otcc.yang.tapi.topology.rev181210.TapiTopologyService; @ExtendWith(MockitoExtension.class) -public class TapiProviderTest extends AbstractTest { - private static NetworkTransactionService networkTransactionService; +public class TapiProviderTest { + @Mock + private DataBroker dataBroker; @Mock private RpcProviderService rpcProviderRegistry; @Mock @@ -43,6 +47,8 @@ public class TapiProviderTest extends AbstractTest { @Mock private NotificationPublishService notificationPublishService; @Mock + private NetworkTransactionService networkTransactionService; + @Mock private OrgOpenroadmServiceService serviceHandler; @Mock private ServiceDataStoreOperations serviceDataStoreOperations; @@ -53,19 +59,17 @@ public class TapiProviderTest extends AbstractTest { @Mock private TapiNetworkModelService tapiNetworkModelServiceImpl; - @BeforeAll - static void setUp() { - networkTransactionService = new NetworkTransactionImpl(getDataBroker()); - } - @Test void testInitRegisterTapiToRpcRegistry() { - TapiProvider provider = new TapiProvider(getDataBroker(), rpcProviderRegistry, notificationService, - notificationPublishService, networkTransactionService, serviceHandler, serviceDataStoreOperations, - tapiNetworkUtils, tapiNetworkModelListenerImpl, tapiNetworkModelServiceImpl); + when(networkTransactionService.read(any(), any())).thenReturn(Futures.immediateFuture(Optional.empty())); + doReturn(emptyFluentFuture()).when(networkTransactionService).commit(); + new TapiProvider(dataBroker, rpcProviderRegistry, notificationService, notificationPublishService, + networkTransactionService, serviceHandler, serviceDataStoreOperations, tapiNetworkUtils, + tapiNetworkModelListenerImpl, tapiNetworkModelServiceImpl); verify(rpcProviderRegistry, times(1)).registerRpcImplementation(any(), any(TapiConnectivityService.class)); verify(rpcProviderRegistry, times(2)).registerRpcImplementation(any(), any(TapiTopologyService.class)); verify(rpcProviderRegistry, times(2)).registerRpcImplementation(any(), any(TapiCommonService.class)); + verify(dataBroker, times(4)).registerDataTreeChangeListener(any(), any()); } } \ No newline at end of file -- 2.36.6