Unregister DataTreeChangeListeners 62/104962/7
authorGilles Thouenon <gilles.thouenon@orange.com>
Fri, 17 Mar 2023 11:23:09 +0000 (12:23 +0100)
committerGilles Thouenon <gilles.thouenon@orange.com>
Fri, 31 Mar 2023 14:04:20 +0000 (16:04 +0200)
- 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 <gilles.thouenon@orange.com>
Change-Id: Ib564b7b74e2b499c129c4c76c0e5a38012853dd7
(cherry picked from commit 3f8fa9fb8da822fbc6579bbaaf49a95b499e9a27)

inventory/src/main/java/org/opendaylight/transportpce/inventory/ListenerProvider.java
networkmodel/src/main/java/org/opendaylight/transportpce/networkmodel/NetworkModelProvider.java
networkmodel/src/test/java/org/opendaylight/transportpce/networkmodel/NetworkModelProviderTest.java
servicehandler/src/test/java/org/opendaylight/transportpce/servicehandler/impl/ServicehandlerProviderTest.java
tapi/src/main/java/org/opendaylight/transportpce/tapi/impl/TapiProvider.java
tapi/src/test/java/org/opendaylight/transportpce/tapi/provider/TapiProviderTest.java

index 7f6578b1b926313eb70fdac28ab9dcb9b2b5ddb7..651621aa59151146af8493cfa9cf08a60bb64c40 100644 (file)
@@ -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<Registration> 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();
+    }
 }
index 571f9076c35ce71e2b8e0fe3a1fd27de8893a883..a1c359452bed1a0c0172a55a1ba5dd54a0e0e94b 100644 (file)
@@ -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<NetConfTopologyListener> dataTreeChangeListenerRegistration;
-    private ListenerRegistration<PortMappingListener> mappingListenerRegistration;
+    private List<Registration> listeners;
     private @NonNull Registration networkutilsServiceRpcRegistration;
     private TpceNetwork tpceNetwork;
     private ListenerRegistration<TransportpceServicehandlerListener> 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();
         }
index 451e63ba55b4666d4bbd34835faa4d00f6339b37..8d5ba9f8c6867aed38f1111c9152721c31728d77 100644 (file)
@@ -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());
     }
 }
index 8a696ab7e2a5f6575d700c85b13249aa54f6978b..0092b0969a70f04cd1d59cac21cfe1b4f11bfef5 100644 (file)
@@ -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
index 8ce95c33956f0c65f4ff2bc78227ad279a5a2726..dc03a1f3e0c0c2933003a51d5bff7381036b5dea 100644 (file)
@@ -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<TapiConnectivityService> rpcRegistration;
     private ObjectRegistration<TransportpceTapinetworkutilsService> tapiNetworkutilsServiceRpcRegistration;
-    private ListenerRegistration<TapiNetconfTopologyListener> dataTreeChangeListenerRegistration;
-    private ListenerRegistration<TapiOrLinkListener> dataTreeChangeListenerRegistration1;
-    private ListenerRegistration<TapiPortMappingListener> mappingListenerListenerRegistration;
+    private List<Registration> listeners;
     private ListenerRegistration<TransportpcePceListener> pcelistenerRegistration;
     private ListenerRegistration<TransportpceRendererListener> rendererlistenerRegistration;
     private ListenerRegistration<TransportpceServicehandlerListener> 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");
     }
 }
index 9427914c4aca2a6b2758ebde1968cf1384c37327..b815d1b5363d8607ea2e0ee91a94d1c89740d434 100644 (file)
@@ -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