Update ListenerRegistry design

This addresses FIXMEs marked for 3.0.0 in ListenerRegistry,
including making it final.

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

diff --git a/common/util/src/main/java/org/opendaylight/yangtools/util/ListenerRegistry.java b/common/util/src/main/java/org/opendaylight/yangtools/util/ListenerRegistry.java
index 56bafdbcce1e0aa3424d656372c3fe39b6ac677a..f44f9f1ceaf7e69822018c72eec1f60a8c024d25 100644 (file)
--- a/common/util/src/main/java/org/opendaylight/yangtools/util/ListenerRegistry.java
+++ b/common/util/src/main/java/org/opendaylight/yangtools/util/ListenerRegistry.java
@@ -12,7 +12,6 @@ import static java.util.Objects.requireNonNull;
 import com.google.common.base.MoreObjects;
 import java.util.Collections;
 import java.util.EventListener;
-import java.util.Iterator;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
@@ -24,8 +23,7 @@ import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.concepts.Mutable;
 
 @ThreadSafe
-//FIXME: 3.0.0: make final, do not implement Iterable
-public class ListenerRegistry<T extends EventListener> implements Iterable<ListenerRegistration<T>>, Mutable {
+public final class ListenerRegistry<T extends EventListener> implements Mutable {
 
     private final Set<ListenerRegistration<? extends T>> listeners = ConcurrentHashMap.newKeySet();
     // This conversion is known to be safe.
@@ -38,27 +36,15 @@ public class ListenerRegistry<T extends EventListener> implements Iterable<Liste
         this.name = name;
     }
 
-    /**
-     * Default constructor.
-     *
-     * @deprecated This class will not be subclassable, use {@link #create()} instead.
-     */
-    @Deprecated
-    public ListenerRegistry() {
-        this(null);
-    }
-
-    public static <T extends EventListener> ListenerRegistry<T> create() {
+    public static <T extends EventListener> @NonNull ListenerRegistry<T> create() {
         return new ListenerRegistry<>(null);
     }
 
-    public static <T extends EventListener> ListenerRegistry<T> create(final @NonNull String name) {
+    public static <T extends EventListener> @NonNull ListenerRegistry<T> create(final @NonNull String name) {
         return new ListenerRegistry<>(requireNonNull(name));
     }
 
-    // FIXME: 3.0.0: return Set<ListenerRegistration<? extends T>>
-    // FIXME: 3.0.0: rename to getRegistrations()
-    public Iterable<ListenerRegistration<T>> getListeners() {
+    public @NonNull Set<? extends ListenerRegistration<? extends T>> getRegistrations() {
         return unmodifiableView;
     }
 
@@ -70,29 +56,15 @@ public class ListenerRegistry<T extends EventListener> implements Iterable<Liste
         return listeners.stream().map(ListenerRegistration::getInstance);
     }
 
-    // FIXME: 3.0.0: change return type to what registerWithType does
-    public ListenerRegistration<T> register(final T listener) {
-        if (listener == null) {
-            throw new IllegalArgumentException("Listener should not be null.");
-        }
-        return registerWithType(listener);
-    }
-
-    // FIXME: 3.0.0: remove this method
-    public <L extends T> ListenerRegistration<L> registerWithType(final L listener) {
+    public <L extends T> @NonNull  ListenerRegistration<L> register(final L listener) {
         final ListenerRegistration<L> ret = new ListenerRegistrationImpl<>(listener, listeners::remove);
         listeners.add(ret);
         return ret;
     }
 
-    @Override
-    public Iterator<ListenerRegistration<T>> iterator() {
-        return unmodifiableView.iterator();
-    }
-
     @Override
     public String toString() {
-        return name == null ? super.toString() : MoreObjects.toStringHelper(this)
+        return MoreObjects.toStringHelper(this).omitNullValues()
                 .add("name", name)
                 .add("size", listeners.size())
                 .toString();
@@ -110,7 +82,7 @@ public class ListenerRegistry<T extends EventListener> implements Iterable<Liste
         @Override
         protected void removeRegistration() {
             removeCall.accept(this);
-            // Do not retail reference to that state
+            // Do not retain reference to that state
             removeCall = null;
         }
     }

diff --git a/common/util/src/test/java/org/opendaylight/yangtools/util/ListenerRegistryTest.java b/common/util/src/test/java/org/opendaylight/yangtools/util/ListenerRegistryTest.java
index 4c2d2ad83a0cae155348424030f3fa934ce433d9..a5996bbc6db705b38730665a2f33f2e2f27f014e 100644 (file)
--- a/common/util/src/test/java/org/opendaylight/yangtools/util/ListenerRegistryTest.java
+++ b/common/util/src/test/java/org/opendaylight/yangtools/util/ListenerRegistryTest.java
@@ -9,11 +9,9 @@ package org.opendaylight.yangtools.util;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 
-import com.google.common.collect.Iterables;
+import com.google.common.collect.ImmutableSet;
 import java.util.EventListener;
-import java.util.Iterator;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -33,50 +31,26 @@ public class ListenerRegistryTest {
     public void init() {
         testEventListener = new TestEventListener() {};
         extendedTestEventListener = new ExtendedTestEventListener() {};
-        registry = new ListenerRegistry<>();
+        registry = ListenerRegistry.create();
     }
 
     @Test
     public void testCreateNewInstance() {
-        assertNotNull("Intance of listener registry shouldn't be null.", registry);
+        assertNotNull("Intance of listener registry should not be null.", registry);
     }
 
     @Test
     public void tetGetListenersMethod() {
-        assertTrue("Listener registry should have any listeners.", Iterables.isEmpty(registry.getListeners()));
+        assertEquals("Listener registry should have any listeners.", ImmutableSet.of(), registry.getRegistrations());
     }
 
     @Test
     public void testRegisterMethod() {
-
-        final ListenerRegistration<TestEventListener> listenerRegistration = registry.register(testEventListener);
-        assertEquals("Listeners should be the same.", testEventListener, listenerRegistration.getInstance());
-
-        expException.expect(IllegalArgumentException.class);
-        expException.expectMessage("Listener should not be null.");
-        registry.register(null);
-    }
-
-    @Test
-    public void testRegisterWithType() {
-        final ListenerRegistration<ExtendedTestEventListener> listenerRegistration = registry.registerWithType(
+        final ListenerRegistration<ExtendedTestEventListener> listenerRegistration = registry.register(
             extendedTestEventListener);
         assertEquals("Listeners should be the same.", extendedTestEventListener, listenerRegistration.getInstance());
     }
 
-    @Test
-    public void testIteratorMethod() {
-        final Iterator<ListenerRegistration<TestEventListener>> listenerIterator = registry.iterator();
-        assertNotNull("Listener iterator shouldn't be null.", listenerIterator);
-    }
-
-    @Test
-    public void testCreateMethod() {
-        final ListenerRegistry<EventListener> emptyRegistry = ListenerRegistry.create();
-        assertTrue("List of listeners in listener registry should be empty.",
-            Iterables.isEmpty(emptyRegistry.getListeners()));
-    }
-
     interface TestEventListener extends EventListener {
 
     }