Remove NotificationListenerRegistration

Replace final user of this specialization with
Registration/AbstractObjectRegistration. This eliminates a bit of
custom code and improves thread safety around concurrent shutdown
and un-registration.

Change-Id: I6ef8bb5e47124c795762e7bba27d73c88651fd64
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/CreateSubscription.java b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscription.java
index c1f2f1556a9a9c43497647af6e2d9feefe7761f0..697c81f55f4b67fac4ad370f548d8fe783ce77bc 100644 (file)
--- a/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscription.java
+++ b/netconf/mdsal-netconf-notification/src/main/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscription.java
@@ -22,11 +22,11 @@ import org.opendaylight.netconf.api.xml.XmlNetconfConstants;
 import org.opendaylight.netconf.mapping.api.SessionAwareNetconfOperation;
 import org.opendaylight.netconf.notifications.NetconfNotificationListener;
 import org.opendaylight.netconf.notifications.NetconfNotificationRegistry;
-import org.opendaylight.netconf.notifications.NotificationListenerRegistration;
 import org.opendaylight.netconf.util.mapping.AbstractSingletonNetconfOperation;
 import org.opendaylight.netconf.util.messages.SubtreeFilter;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.notification._1._0.rev080714.CreateSubscriptionInput;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.notification._1._0.rev080714.StreamNameType;
+import org.opendaylight.yangtools.concepts.Registration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
@@ -45,7 +45,7 @@ public class CreateSubscription extends AbstractSingletonNetconfOperation
     static final String CREATE_SUBSCRIPTION = "create-subscription";
 
     private final NetconfNotificationRegistry notifications;
-    private final List<NotificationListenerRegistration> subscriptions = new ArrayList<>();
+    private final List<Registration> subscriptions = new ArrayList<>();
     private NetconfSession netconfSession;
 
     public CreateSubscription(final String netconfSessionIdForReporting,
@@ -84,9 +84,8 @@ public class CreateSubscription extends AbstractSingletonNetconfOperation
                     getNetconfSessionIdForReporting());
         }
 
-        final NotificationListenerRegistration notificationListenerRegistration = notifications
-                .registerNotificationListener(streamNameType, new NotificationSubscription(netconfSession, filter));
-        subscriptions.add(notificationListenerRegistration);
+        subscriptions.add(notifications.registerNotificationListener(streamNameType,
+            new NotificationSubscription(netconfSession, filter)));
 
         return document.createElement(XmlNetconfConstants.OK);
     }
@@ -116,9 +115,8 @@ public class CreateSubscription extends AbstractSingletonNetconfOperation
     public void close() {
         netconfSession = null;
         // Unregister from notification streams
-        for (final NotificationListenerRegistration subscription : subscriptions) {
-            subscription.close();
-        }
+        subscriptions.forEach(Registration::close);
+        subscriptions.clear();
     }
 
     private static class NotificationSubscription implements NetconfNotificationListener {

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 5e2cc131405e9dc201dd6aef6cb48aff16c3d990..b7b0680e99501bd7d082c482f02fadb5cace3731 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
@@ -19,6 +19,7 @@ import com.google.common.collect.Multiset;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import javax.annotation.PreDestroy;
@@ -33,7 +34,6 @@ import org.opendaylight.netconf.notifications.BaseNotificationPublisherRegistrat
 import org.opendaylight.netconf.notifications.NetconfNotificationCollector;
 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.YangLibraryPublisherRegistration;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.notification._1._0.rev080714.StreamNameType;
@@ -84,8 +84,7 @@ public final class NetconfNotificationManager implements NetconfNotificationColl
     // since the listeners/publishers can block the whole notification processing
 
     @GuardedBy("this")
-    private final Multimap<StreamNameType, GenericNotificationListenerReg> notificationListeners =
-            HashMultimap.create();
+    private final Multimap<StreamNameType, ListenerReg> notificationListeners = HashMultimap.create();
 
     @GuardedBy("this")
     private final Set<StreamListenerReg> streamListeners = new HashSet<>();
@@ -113,10 +112,8 @@ public final class NetconfNotificationManager implements NetconfNotificationColl
     @Override
     public synchronized void close() {
         // Unregister all listeners
-        // Use new list to avoid ConcurrentModificationException
-        for (final GenericNotificationListenerReg listenerReg : new ArrayList<>(notificationListeners.values())) {
-            listenerReg.close();
-        }
+        // Use new list to avoid ConcurrentModificationException when the registration removes itself
+        List.copyOf(notificationListeners.values()).forEach(ListenerReg::close);
         notificationListeners.clear();
 
         // Unregister all publishers
@@ -137,30 +134,16 @@ public final class NetconfNotificationManager implements NetconfNotificationColl
             LOG.debug("Notification of type {} detected: {}", stream, notification);
         }
 
-        for (final GenericNotificationListenerReg listenerReg : notificationListeners.get(stream)) {
-            listenerReg.getListener().onNotification(stream, notification);
+        for (var listenerReg : notificationListeners.get(stream)) {
+            listenerReg.getInstance().onNotification(stream, notification);
         }
     }
 
     @Override
-    public synchronized NotificationListenerRegistration registerNotificationListener(
-            final StreamNameType stream,
+    public synchronized Registration registerNotificationListener(final StreamNameType stream,
             final NetconfNotificationListener listener) {
-        requireNonNull(stream);
-        requireNonNull(listener);
-
+        final var reg = new ListenerReg(listener, stream);
         LOG.trace("Notification listener registered for stream: {}", stream);
-
-        final GenericNotificationListenerReg reg = new GenericNotificationListenerReg(listener, stream) {
-            @Override
-            public void close() {
-                synchronized (NetconfNotificationManager.this) {
-                    LOG.trace("Notification listener unregistered for stream: {}", stream);
-                    super.close();
-                }
-            }
-        };
-
         notificationListeners.put(stream, reg);
         return reg;
     }
@@ -359,23 +342,20 @@ public final class NetconfNotificationManager implements NetconfNotificationColl
         }
     }
 
-    private class GenericNotificationListenerReg implements NotificationListenerRegistration {
-        private final NetconfNotificationListener listener;
-        private final StreamNameType listenedStream;
+    private final class ListenerReg extends AbstractObjectRegistration<NetconfNotificationListener> {
+        private final StreamNameType stream;
 
-        GenericNotificationListenerReg(final NetconfNotificationListener listener,
-                                       final StreamNameType listenedStream) {
-            this.listener = listener;
-            this.listenedStream = listenedStream;
-        }
-
-        public NetconfNotificationListener getListener() {
-            return listener;
+        ListenerReg(final @NonNull NetconfNotificationListener instance, final @NonNull StreamNameType stream) {
+            super(instance);
+            this.stream = requireNonNull(stream);
         }
 
         @Override
-        public void close() {
-            notificationListeners.remove(listenedStream, this);
+        protected void removeRegistration() {
+            synchronized (NetconfNotificationManager.this) {
+                LOG.trace("Notification listener unregistered for stream: {}", stream);
+                notificationListeners.remove(stream, this);
+            }
         }
     }
 

diff --git a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscriptionTest.java b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscriptionTest.java
index 3d79f8644619db0cde18435538be49fe2697ce3c..4a9bc62e98d93718c4acf88060ab472d291d790f 100644 (file)
--- a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscriptionTest.java
+++ b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/CreateSubscriptionTest.java
@@ -23,8 +23,8 @@ import org.opendaylight.netconf.api.xml.XmlElement;
 import org.opendaylight.netconf.api.xml.XmlUtil;
 import org.opendaylight.netconf.notifications.NetconfNotificationListener;
 import org.opendaylight.netconf.notifications.NetconfNotificationRegistry;
-import org.opendaylight.netconf.notifications.NotificationListenerRegistration;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.notification._1._0.rev080714.StreamNameType;
+import org.opendaylight.yangtools.concepts.Registration;
 import org.w3c.dom.Element;
 
 @RunWith(MockitoJUnitRunner.StrictStubs.class)
@@ -42,7 +42,7 @@ public class CreateSubscriptionTest {
     @Before
     public void setUp() {
         doReturn(true).when(notificationRegistry).isStreamAvailable(any(StreamNameType.class));
-        doReturn(mock(NotificationListenerRegistration.class)).when(notificationRegistry)
+        doReturn(mock(Registration.class)).when(notificationRegistry)
                 .registerNotificationListener(any(StreamNameType.class), any(NetconfNotificationListener.class));
     }
 

diff --git a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManagerTest.java b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManagerTest.java
index 34665252ef975f9c2150207f587d18f3ff4f300b..a554c0d7f6ba53d46f6e5ac43973cca4fff02fc8 100644 (file)
--- a/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManagerTest.java
+++ b/netconf/mdsal-netconf-notification/src/test/java/org/opendaylight/netconf/mdsal/notification/impl/NetconfNotificationManagerTest.java
@@ -32,7 +32,6 @@ import org.opendaylight.netconf.api.xml.XmlUtil;
 import org.opendaylight.netconf.notifications.BaseNotificationPublisherRegistration;
 import org.opendaylight.netconf.notifications.NetconfNotificationCollector;
 import org.opendaylight.netconf.notifications.NetconfNotificationListener;
-import org.opendaylight.netconf.notifications.NotificationListenerRegistration;
 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.yang.gen.v1.urn.ietf.params.xml.ns.netmod.notification.rev080714.netconf.streams.StreamBuilder;
@@ -130,14 +129,15 @@ public class NetconfNotificationManagerTest {
 
         final NetconfNotificationListener listener = mock(NetconfNotificationListener.class);
         doNothing().when(listener).onNotification(any(StreamNameType.class), any(NotificationMessage.class));
-        final NotificationListenerRegistration notificationListenerRegistration = netconfNotificationManager
-                .registerNotificationListener(NetconfNotificationManager.BASE_NETCONF_STREAM.getName(), listener);
+
         final NetconfCapabilityChange notification = capabilityChangedBuilder.build();
-        baseNotificationPublisherRegistration.onCapabilityChanged(notification);
 
-        verify(listener).onNotification(any(StreamNameType.class), any(NotificationMessage.class));
+        try (var reg = netconfNotificationManager.registerNotificationListener(
+                NetconfNotificationManager.BASE_NETCONF_STREAM.getName(), listener)) {
+            baseNotificationPublisherRegistration.onCapabilityChanged(notification);
 
-        notificationListenerRegistration.close();
+            verify(listener).onNotification(any(StreamNameType.class), any(NotificationMessage.class));
+        }
 
         baseNotificationPublisherRegistration.onCapabilityChanged(notification);
         verifyNoMoreInteractions(listener);

diff --git a/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NetconfNotificationRegistry.java b/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NetconfNotificationRegistry.java
index 1831669cb953166e580a1b27a4acd91facb7cdde..2eb729e8fa77f866dae06783127fe032b32122bc 100644 (file)
--- a/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NetconfNotificationRegistry.java
+++ b/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NetconfNotificationRegistry.java
@@ -7,15 +7,17 @@
  */
 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;
+import org.opendaylight.yangtools.concepts.Registration;
 
 public interface NetconfNotificationRegistry {
     /**
      * Add listener for a certain notification type.
      */
-    NotificationListenerRegistration registerNotificationListener(StreamNameType stream,
-                                                                  NetconfNotificationListener listener);
+    @NonNull Registration registerNotificationListener(@NonNull StreamNameType stream,
+        @NonNull NetconfNotificationListener listener);
 
     /**
      * Check stream availability.

diff --git a/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NotificationListenerRegistration.java b/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NotificationListenerRegistration.java
deleted file mode 100644 (file)
index 15a6753..0000000
--- a/netconf/netconf-notifications-api/src/main/java/org/opendaylight/netconf/notifications/NotificationListenerRegistration.java
+++ /dev/null
@@ -1,15 +0,0 @@
-/*
- * Copyright (c) 2015 Cisco Systems, Inc. and others.  All rights reserved.
- *
- * This program and the accompanying materials are made available under the
- * terms of the Eclipse Public License v1.0 which accompanies this distribution,
- * and is available at http://www.eclipse.org/legal/epl-v10.html
- */
-package org.opendaylight.netconf.notifications;
-
-/**
- * Manages the registration of a single listener.
- */
-public interface NotificationListenerRegistration extends NotificationRegistration {
-
-}