NetconfMonitoringService should use Registration

We are using raw AutoCloseable here, whereas we should be using simple
registrations. It also flushes out a few places where we fail to clean
up registrations.

Change-Id: I60f956f7e7dd9b394834ae046069d0112d496975
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/netconf/mdsal-netconf-monitoring/src/main/java/org/opendaylight/controller/config/yang/netconf/mdsal/monitoring/MonitoringToMdsalWriter.java b/netconf/mdsal-netconf-monitoring/src/main/java/org/opendaylight/controller/config/yang/netconf/mdsal/monitoring/MonitoringToMdsalWriter.java
index 09aa2d1f26edb3b66e56573a796f8df6514296fc..e6b0fd6b06ed96eb1166fb41ed1d7c475cf7a166 100644 (file)
--- a/netconf/mdsal-netconf-monitoring/src/main/java/org/opendaylight/controller/config/yang/netconf/mdsal/monitoring/MonitoringToMdsalWriter.java
+++ b/netconf/mdsal-netconf-monitoring/src/main/java/org/opendaylight/controller/config/yang/netconf/mdsal/monitoring/MonitoringToMdsalWriter.java
@@ -50,12 +50,21 @@ public final class MonitoringToMdsalWriter implements AutoCloseable, NetconfMoni
         this.dataBroker = dataBroker;
     }
 
+    /**
+     * Invoked using blueprint.
+     */
+    public void start() {
+        // FIXME: close registrations
+        serverMonitoringDependency.registerCapabilitiesListener(this);
+        serverMonitoringDependency.registerSessionsListener(this);
+    }
+
     /**
      * Invoked using blueprint.
      */
     @Override
     public void close() {
-        runTransaction((tx) -> tx.delete(LogicalDatastoreType.OPERATIONAL,
+        runTransaction(tx -> tx.delete(LogicalDatastoreType.OPERATIONAL,
                 InstanceIdentifier.create(NetconfState.class)));
     }
 
@@ -63,38 +72,30 @@ public final class MonitoringToMdsalWriter implements AutoCloseable, NetconfMoni
     public void onSessionStarted(final Session session) {
         final InstanceIdentifier<Session> sessionPath =
                 SESSIONS_INSTANCE_IDENTIFIER.child(Session.class, session.key());
-        runTransaction((tx) -> tx.put(LogicalDatastoreType.OPERATIONAL, sessionPath, session));
+        runTransaction(tx -> tx.put(LogicalDatastoreType.OPERATIONAL, sessionPath, session));
     }
 
     @Override
     public void onSessionEnded(final Session session) {
         final InstanceIdentifier<Session> sessionPath =
                 SESSIONS_INSTANCE_IDENTIFIER.child(Session.class, session.key());
-        runTransaction((tx) -> tx.delete(LogicalDatastoreType.OPERATIONAL, sessionPath));
+        runTransaction(tx -> tx.delete(LogicalDatastoreType.OPERATIONAL, sessionPath));
     }
 
     @Override
     public void onSessionsUpdated(final Collection<Session> sessions) {
-        runTransaction((tx) -> updateSessions(tx, sessions));
+        runTransaction(tx -> updateSessions(tx, sessions));
     }
 
     @Override
     public void onCapabilitiesChanged(final Capabilities capabilities) {
-        runTransaction((tx) -> tx.put(LogicalDatastoreType.OPERATIONAL, CAPABILITIES_INSTANCE_IDENTIFIER,
+        runTransaction(tx -> tx.put(LogicalDatastoreType.OPERATIONAL, CAPABILITIES_INSTANCE_IDENTIFIER,
                 capabilities));
     }
 
     @Override
     public void onSchemasChanged(final Schemas schemas) {
-        runTransaction((tx) -> tx.put(LogicalDatastoreType.OPERATIONAL, SCHEMAS_INSTANCE_IDENTIFIER, schemas));
-    }
-
-    /**
-     * Invoked using blueprint.
-     */
-    public void start() {
-        serverMonitoringDependency.registerCapabilitiesListener(this);
-        serverMonitoringDependency.registerSessionsListener(this);
+        runTransaction(tx -> tx.put(LogicalDatastoreType.OPERATIONAL, SCHEMAS_INSTANCE_IDENTIFIER, schemas));
     }
 
     private void runTransaction(final Consumer<WriteTransaction> txUser) {

diff --git a/netconf/tools/netconf-testtool/src/main/java/org/opendaylight/netconf/test/tool/DummyMonitoringService.java b/netconf/tools/netconf-testtool/src/main/java/org/opendaylight/netconf/test/tool/DummyMonitoringService.java
index d3b185d2451b9dce70391bd76b0a494abfb457ae..738079920a501900e6257bda17c9b6c96a6cd38e 100644 (file)
--- a/netconf/tools/netconf-testtool/src/main/java/org/opendaylight/netconf/test/tool/DummyMonitoringService.java
+++ b/netconf/tools/netconf-testtool/src/main/java/org/opendaylight/netconf/test/tool/DummyMonitoringService.java
@@ -36,6 +36,8 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.mon
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.schemas.Schema.Location.Enumeration;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.schemas.SchemaBuilder;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.schemas.SchemaKey;
+import org.opendaylight.yangtools.concepts.NoOpObjectRegistration;
+import org.opendaylight.yangtools.concepts.Registration;
 
 public class DummyMonitoringService implements NetconfMonitoringService {
 
@@ -133,13 +135,12 @@ public class DummyMonitoringService implements NetconfMonitoringService {
     }
 
     @Override
-    public AutoCloseable registerCapabilitiesListener(final CapabilitiesListener listener) {
-        return null;
+    public Registration registerCapabilitiesListener(final CapabilitiesListener listener) {
+        return NoOpObjectRegistration.of(listener);
     }
 
     @Override
-    public AutoCloseable registerSessionsListener(final SessionsListener listener) {
-        return null;
+    public Registration registerSessionsListener(final SessionsListener listener) {
+        return NoOpObjectRegistration.of(listener);
     }
-
 }

diff --git a/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/monitoring/NetconfMonitoringService.java b/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/monitoring/NetconfMonitoringService.java
index 493052bb5013bc926ef9566bdce893ea2f03606b..0d1ed31f818d8ab57a1307df40471415c3ed5463 100644 (file)
--- a/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/monitoring/NetconfMonitoringService.java
+++ b/protocol/netconf-api/src/main/java/org/opendaylight/netconf/api/monitoring/NetconfMonitoringService.java
@@ -13,6 +13,7 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.mon
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.Schemas;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.Sessions;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.sessions.Session;
+import org.opendaylight.yangtools.concepts.Registration;
 
 public interface NetconfMonitoringService {
 
@@ -39,7 +40,7 @@ public interface NetconfMonitoringService {
      * @param listener Monitoring listener
      * @return listener registration
      */
-    AutoCloseable registerCapabilitiesListener(CapabilitiesListener listener);
+    Registration registerCapabilitiesListener(CapabilitiesListener listener);
 
     /**
      * Allows push based sessions information transfer.
@@ -47,7 +48,7 @@ public interface NetconfMonitoringService {
      * @param listener Monitoring listener
      * @return listener registration
      */
-    AutoCloseable registerSessionsListener(SessionsListener listener);
+    Registration registerSessionsListener(SessionsListener listener);
 
     interface CapabilitiesListener {
 

diff --git a/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfCapabilityMonitoringService.java b/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfCapabilityMonitoringService.java
index 5cc087248a331deb0e61a8b87f84ab5c05e16118..c04253e0627432f23e1282ab5c3063b1ce713a3c 100644 (file)
--- a/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfCapabilityMonitoringService.java
+++ b/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfCapabilityMonitoringService.java
@@ -41,6 +41,8 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.not
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.notifications.rev120206.NetconfCapabilityChangeBuilder;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.notifications.rev120206.changed.by.parms.ChangedByBuilder;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.notifications.rev120206.changed.by.parms.changed.by.server.or.user.ServerBuilder;
+import org.opendaylight.yangtools.concepts.AbstractRegistration;
+import org.opendaylight.yangtools.concepts.Registration;
 import org.opendaylight.yangtools.yang.common.Empty;
 
 final class NetconfCapabilityMonitoringService implements CapabilityListener, AutoCloseable {
@@ -127,13 +129,16 @@ final class NetconfCapabilityMonitoringService implements CapabilityListener, Au
         return new CapabilitiesBuilder().setCapability(Set.copyOf(capabilities.keySet())).build();
     }
 
-    synchronized AutoCloseable registerListener(final NetconfMonitoringService.CapabilitiesListener listener) {
+    synchronized Registration registerListener(final NetconfMonitoringService.CapabilitiesListener listener) {
         listeners.add(listener);
         listener.onCapabilitiesChanged(getCapabilities());
         listener.onSchemasChanged(getSchemas());
-        return () -> {
-            synchronized (NetconfCapabilityMonitoringService.this) {
-                listeners.remove(listener);
+        return new AbstractRegistration() {
+            @Override
+            protected void removeRegistration() {
+                synchronized (NetconfCapabilityMonitoringService.this) {
+                    listeners.remove(listener);
+                }
             }
         };
     }

diff --git a/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImpl.java b/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImpl.java
index 90bcfe3bd55561a713bbff63e2d41b2363974d53..d60bf5a6fb3d221b94e7544099660b5a1cb867b4 100644 (file)
--- a/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImpl.java
+++ b/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfMonitoringServiceImpl.java
@@ -16,27 +16,28 @@ import org.opendaylight.netconf.notifications.BaseNotificationPublisherRegistrat
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.Capabilities;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.Schemas;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.Sessions;
+import org.opendaylight.yangtools.concepts.Registration;
 
 public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, AutoCloseable {
 
     private final NetconfCapabilityMonitoringService capabilityMonitoring;
     private final NetconfSessionMonitoringService sessionMonitoring;
 
-    public NetconfMonitoringServiceImpl(NetconfOperationServiceFactory opProvider) {
+    public NetconfMonitoringServiceImpl(final NetconfOperationServiceFactory opProvider) {
         this(opProvider, Optional.empty(), 0);
     }
 
-    public NetconfMonitoringServiceImpl(NetconfOperationServiceFactory opProvider,
-                                        ScheduledThreadPool threadPool,
-                                        long updateInterval) {
+    public NetconfMonitoringServiceImpl(final NetconfOperationServiceFactory opProvider,
+                                        final ScheduledThreadPool threadPool,
+                                        final long updateInterval) {
         this(opProvider, Optional.ofNullable(threadPool), updateInterval);
     }
 
-    public NetconfMonitoringServiceImpl(NetconfOperationServiceFactory opProvider,
-                                        Optional<ScheduledThreadPool> threadPool,
-                                        long updateInterval) {
-        this.capabilityMonitoring = new NetconfCapabilityMonitoringService(opProvider);
-        this.sessionMonitoring = new NetconfSessionMonitoringService(threadPool, updateInterval);
+    public NetconfMonitoringServiceImpl(final NetconfOperationServiceFactory opProvider,
+                                        final Optional<ScheduledThreadPool> threadPool,
+                                        final long updateInterval) {
+        capabilityMonitoring = new NetconfCapabilityMonitoringService(opProvider);
+        sessionMonitoring = new NetconfSessionMonitoringService(threadPool, updateInterval);
 
     }
 
@@ -56,7 +57,7 @@ public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, A
     }
 
     @Override
-    public String getSchemaForCapability(String moduleName, Optional<String> revision) {
+    public String getSchemaForCapability(final String moduleName, final Optional<String> revision) {
         return capabilityMonitoring.getSchemaForModuleRevision(moduleName, revision);
     }
 
@@ -66,17 +67,17 @@ public class NetconfMonitoringServiceImpl implements NetconfMonitoringService, A
     }
 
     @Override
-    public AutoCloseable registerCapabilitiesListener(CapabilitiesListener listener) {
+    public Registration registerCapabilitiesListener(final CapabilitiesListener listener) {
         return capabilityMonitoring.registerListener(listener);
     }
 
     @Override
-    public AutoCloseable registerSessionsListener(SessionsListener listener) {
+    public Registration registerSessionsListener(final SessionsListener listener) {
         return sessionMonitoring.registerListener(listener);
     }
 
-    public void setNotificationPublisher(BaseNotificationPublisherRegistration notificationPublisher) {
-        this.capabilityMonitoring.setNotificationPublisher(notificationPublisher);
+    public void setNotificationPublisher(final BaseNotificationPublisherRegistration notificationPublisher) {
+        capabilityMonitoring.setNotificationPublisher(notificationPublisher);
     }
 
     @Override

diff --git a/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfSessionMonitoringService.java b/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfSessionMonitoringService.java
index df39aa11ae8d789a9b8e2df2bab4eb3281cecaa5..09dabf5464531a5d837a862fb681a8d2c7a26df2 100644 (file)
--- a/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfSessionMonitoringService.java
+++ b/protocol/netconf-impl/src/main/java/org/opendaylight/netconf/impl/osgi/NetconfSessionMonitoringService.java
@@ -27,6 +27,8 @@ import org.opendaylight.netconf.api.monitoring.SessionListener;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.Sessions;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.SessionsBuilder;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.netconf.monitoring.rev101004.netconf.state.sessions.Session;
+import org.opendaylight.yangtools.concepts.AbstractRegistration;
+import org.opendaylight.yangtools.concepts.Registration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -56,13 +58,13 @@ class NetconfSessionMonitoringService implements SessionListener, AutoCloseable
             final long updateInterval) {
         this.updateInterval = updateInterval;
         if (schedulingThreadPool.isPresent() && updateInterval > 0) {
-            this.executor = schedulingThreadPool.get().getExecutor();
+            executor = schedulingThreadPool.get().getExecutor();
             LOG.info("/netconf-state/sessions will be updated every {} seconds.", updateInterval);
         } else {
             LOG.info("Scheduling thread pool is present = {}, "
                     + "update interval {}: /netconf-state/sessions won't be updated.",
                     schedulingThreadPool.isPresent(), updateInterval);
-            this.executor = null;
+            executor = null;
         }
     }
 
@@ -96,12 +98,17 @@ class NetconfSessionMonitoringService implements SessionListener, AutoCloseable
         changedSessions.add(event.getSession());
     }
 
-    synchronized AutoCloseable registerListener(final NetconfMonitoringService.SessionsListener listener) {
+    synchronized Registration registerListener(final NetconfMonitoringService.SessionsListener listener) {
         listeners.add(listener);
         if (!running) {
             startUpdateSessionStats();
         }
-        return () -> listeners.remove(listener);
+        return new AbstractRegistration() {
+            @Override
+            protected void removeRegistration() {
+                listeners.remove(listener);
+            }
+        };
     }
 
     @Override

diff --git a/protocol/netconf-impl/src/test/java/org/opendaylight/netconf/impl/ConcurrentClientsTest.java b/protocol/netconf-impl/src/test/java/org/opendaylight/netconf/impl/ConcurrentClientsTest.java
index f533e6ab1ad3d775c1a980a23c024fc83daf91ce..56d17b233a4416c233a40a429c132aa31752f87c 100644 (file)
--- a/protocol/netconf-impl/src/test/java/org/opendaylight/netconf/impl/ConcurrentClientsTest.java
+++ b/protocol/netconf-impl/src/test/java/org/opendaylight/netconf/impl/ConcurrentClientsTest.java
@@ -124,9 +124,8 @@ public class ConcurrentClientsTest {
         doNothing().when(sessionListener).onSessionUp(any(NetconfServerSession.class));
         doNothing().when(sessionListener).onSessionDown(any(NetconfServerSession.class));
         doNothing().when(sessionListener).onSessionEvent(any(SessionEvent.class));
-        doReturn((AutoCloseable) () -> {
-
-        }).when(monitoring).registerCapabilitiesListener(any(NetconfMonitoringService.CapabilitiesListener.class));
+        doReturn((Registration) () -> { }).when(monitoring)
+            .registerCapabilitiesListener(any(NetconfMonitoringService.CapabilitiesListener.class));
         doReturn(sessionListener).when(monitoring).getSessionListener();
         doReturn(new CapabilitiesBuilder().setCapability(Set.of()).build()).when(monitoring).getCapabilities();
         return monitoring;