Disconnect NetconfNotificationCollector registration

NotificationRegistration does is actually the same thing as
Registration, except it does not enforce idempotence. Replace its use in
NetconfNotificationCollector with Registration and use an explicit
AbstractObjectRegistration subclass.

This fixes potential problems with multiple (un)registrations of the
same object and opens up the possibility for future removal of locking,
as AbstractObjectRegistration knows whether it is closed or not.

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

diff --git a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManager.java b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManager.java
index 6d3d61fb737ed65e601b91cd3b76358dd368602f..5e2cc131405e9dc201dd6aef6cb48aff16c3d990 100644 (file)
--- a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManager.java
+++ b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManager.java
@@ -25,6 +25,7 @@ import javax.annotation.PreDestroy;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import org.checkerframework.checker.lock.qual.GuardedBy;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.binding.dom.codec.spi.BindingDOMCodecFactory;
 import org.opendaylight.mdsal.binding.runtime.api.BindingRuntimeGenerator;
 import org.opendaylight.netconf.api.messages.NotificationMessage;
@@ -34,7 +35,6 @@ import org.opendaylight.netconf.notifications.NetconfNotificationListener;
 import org.opendaylight.netconf.notifications.NetconfNotificationRegistry;
 import org.opendaylight.netconf.notifications.NotificationListenerRegistration;
 import org.opendaylight.netconf.notifications.NotificationPublisherRegistration;
-import org.opendaylight.netconf.notifications.NotificationRegistration;
 import org.opendaylight.netconf.notifications.YangLibraryPublisherRegistration;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.notification._1._0.rev080714.StreamNameType;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.Streams;
@@ -47,6 +47,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.NetconfSessionStart;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.library.rev190104.YangLibraryChange;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.library.rev190104.YangLibraryUpdate;
+import org.opendaylight.yangtools.concepts.AbstractObjectRegistration;
+import org.opendaylight.yangtools.concepts.Registration;
 import org.opendaylight.yangtools.yang.binding.Notification;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
 import org.opendaylight.yangtools.yang.parser.api.YangParserException;
@@ -86,7 +88,7 @@ public final class NetconfNotificationManager implements NetconfNotificationColl
             HashMultimap.create();
 
     @GuardedBy("this")
-    private final Set<NetconfNotificationStreamListener> streamListeners = new HashSet<>();
+    private final Set<StreamListenerReg> streamListeners = new HashSet<>();
 
     @GuardedBy("this")
     private final Map<StreamNameType, Stream> streamMetadata = new HashMap<>();
@@ -174,20 +176,16 @@ public final class NetconfNotificationManager implements NetconfNotificationColl
     }
 
     @Override
-    public synchronized NotificationRegistration registerStreamListener(
-            final NetconfNotificationStreamListener listener) {
-        streamListeners.add(listener);
+    public synchronized Registration registerStreamListener(final NetconfNotificationStreamListener listener) {
+        final var reg = new StreamListenerReg(listener);
+        streamListeners.add(reg);
 
         // Notify about all already available
-        for (final Stream availableStream : streamMetadata.values()) {
+        for (var availableStream : streamMetadata.values()) {
             listener.onStreamRegistered(availableStream);
         }
 
-        return () -> {
-            synchronized (NetconfNotificationManager.this) {
-                streamListeners.remove(listener);
-            }
-        };
+        return reg;
     }
 
     @Override
@@ -240,14 +238,14 @@ public final class NetconfNotificationManager implements NetconfNotificationColl
     }
 
     private synchronized void notifyStreamAdded(final Stream stream) {
-        for (final NetconfNotificationStreamListener streamListener : streamListeners) {
-            streamListener.onStreamRegistered(stream);
+        for (var streamListener : streamListeners) {
+            streamListener.getInstance().onStreamRegistered(stream);
         }
     }
 
     private synchronized void notifyStreamRemoved(final StreamNameType stream) {
-        for (final NetconfNotificationStreamListener streamListener : streamListeners) {
-            streamListener.onStreamUnregistered(stream);
+        for (var streamListener : streamListeners) {
+            streamListener.getInstance().onStreamUnregistered(stream);
         }
     }
 
@@ -380,4 +378,17 @@ public final class NetconfNotificationManager implements NetconfNotificationColl
             notificationListeners.remove(listenedStream, this);
         }
     }
+
+    private final class StreamListenerReg extends AbstractObjectRegistration<NetconfNotificationStreamListener> {
+        StreamListenerReg(final @NonNull NetconfNotificationStreamListener instance) {
+            super(instance);
+        }
+
+        @Override
+        protected void removeRegistration() {
+            synchronized (NetconfNotificationManager.this) {
+                streamListeners.remove(this);
+            }
+        }
+    }
 }

diff --git a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationToMdsalWriter.java b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationToMdsalWriter.java
index a78e08c44aae949eed414601e2727bf242415619..36b1af9fcbab163f46adbb099e32af983a1babd3 100644 (file)
--- a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationToMdsalWriter.java
+++ b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationToMdsalWriter.java
@@ -18,12 +18,12 @@ import org.opendaylight.mdsal.common.api.CommitInfo;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.netconf.notifications.NetconfNotificationCollector;
 import org.opendaylight.netconf.notifications.NetconfNotificationCollector.NetconfNotificationStreamListener;
-import org.opendaylight.netconf.notifications.NotificationRegistration;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.notification._1._0.rev080714.StreamNameType;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.Netconf;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.Streams;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.streams.Stream;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.streams.StreamKey;
+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;
@@ -42,7 +42,7 @@ public final class NotificationToMdsalWriter implements NetconfNotificationStrea
         InstanceIdentifier.builder(Netconf.class).child(Streams.class).build();
 
     private final DataBroker dataBroker;
-    private final NotificationRegistration notificationRegistration;
+    private final Registration notificationRegistration;
 
     @Activate
     public NotificationToMdsalWriter(

diff --git a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationToMdsalWriterTest.java b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationToMdsalWriterTest.java
index 5d43bb4e519f323bdd3718ce6e3c3f286db4b39c..2e626fa4140d7f64d653eef2eb9c36263f1d3883 100644 (file)
--- a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationToMdsalWriterTest.java
+++ b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NotificationToMdsalWriterTest.java
@@ -23,37 +23,32 @@ import org.opendaylight.mdsal.binding.api.DataBroker;
 import org.opendaylight.mdsal.binding.api.WriteTransaction;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.netconf.notifications.NetconfNotificationCollector;
-import org.opendaylight.netconf.notifications.NotificationRegistration;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.notification._1._0.rev080714.StreamNameType;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.Netconf;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.Streams;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.streams.Stream;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.streams.StreamBuilder;
-import org.opendaylight.yangtools.yang.binding.DataObject;
+import org.opendaylight.yangtools.concepts.Registration;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
 @RunWith(MockitoJUnitRunner.StrictStubs.class)
 public class NotificationToMdsalWriterTest {
-
     @Mock
     private DataBroker dataBroker;
+    @Mock
+    private Registration registration;
 
     private NotificationToMdsalWriter writer;
 
-    @Mock
-    private NotificationRegistration notificationRegistration;
-
     @Before
     public void setUp() {
         final NetconfNotificationCollector notificationCollector = mock(NetconfNotificationCollector.class);
 
-        doReturn(notificationRegistration).when(notificationCollector).registerStreamListener(any(
-                NetconfNotificationCollector.NetconfNotificationStreamListener.class));
+        doReturn(registration).when(notificationCollector).registerStreamListener(any());
 
         WriteTransaction tx = mock(WriteTransaction.class);
-        doNothing().when(tx).merge(any(LogicalDatastoreType.class), any(InstanceIdentifier.class),
-            any(DataObject.class));
-        doNothing().when(tx).delete(any(LogicalDatastoreType.class), any(InstanceIdentifier.class));
+        doNothing().when(tx).merge(any(), any(), any());
+        doNothing().when(tx).delete(any(), any());
         doReturn(emptyFluentFuture()).when(tx).commit();
         doReturn(tx).when(dataBroker).newWriteOnlyTransaction();
 
@@ -62,9 +57,9 @@ public class NotificationToMdsalWriterTest {
 
     @Test
     public void testStreamRegisteration() {
-        final StreamNameType testStreamName = new StreamNameType("TESTSTREAM");
-        final Stream testStream = new StreamBuilder().setName(testStreamName).build();
-        final InstanceIdentifier<Stream> streamIdentifier = InstanceIdentifier.create(Netconf.class)
+        final var testStreamName = new StreamNameType("TESTSTREAM");
+        final var testStream = new StreamBuilder().setName(testStreamName).build();
+        final var streamIdentifier = InstanceIdentifier.create(Netconf.class)
                 .child(Streams.class).child(Stream.class, testStream.key());
 
         writer.onStreamRegistered(testStream);
@@ -79,14 +74,13 @@ public class NotificationToMdsalWriterTest {
 
     @Test
     public void testClose() {
-        doNothing().when(notificationRegistration).close();
+        doNothing().when(registration).close();
 
-        final InstanceIdentifier<Netconf> streamIdentifier = InstanceIdentifier.create(Netconf.class);
+        final var streamIdentifier = InstanceIdentifier.create(Netconf.class);
 
         writer.close();
 
         verify(dataBroker.newWriteOnlyTransaction()).delete(LogicalDatastoreType.OPERATIONAL, streamIdentifier);
-        verify(notificationRegistration).close();
+        verify(registration).close();
     }
-
 }

diff --git a/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NetconfNotificationCollector.java b/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NetconfNotificationCollector.java
index b073f87bfbf62b45e5d4ff7c758e425ca41aa2a5..e1733427888feca2eb691ffeedaf404bbeecc64e 100644 (file)
--- a/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NetconfNotificationCollector.java
+++ b/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NetconfNotificationCollector.java
@@ -7,8 +7,10 @@
  */
 package org.opendaylight.netconf.notifications;
 
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.notification._1._0.rev080714.StreamNameType;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.streams.Stream;
+import org.opendaylight.yangtools.concepts.Registration;
 
 /**
  * Collector of all notifications. Base or generic.
@@ -40,7 +42,7 @@ public interface NetconfNotificationCollector {
      * <p>
      * The listener should receive callbacks for each stream available prior to the registration when its registered.
      */
-    NotificationRegistration registerStreamListener(NetconfNotificationStreamListener listener);
+    @NonNull Registration registerStreamListener(@NonNull NetconfNotificationStreamListener listener);
 
     /**
      * Simple listener that receives notifications about changes in stream availability.