Ensure AbstractUntypedActor#unknownMessage() is called 17/36117/9
authorRobert Varga <rovarga@cisco.com>
Fri, 11 Mar 2016 12:20:53 +0000 (13:20 +0100)
committerTom Pantelis <tpanteli@brocade.com>
Tue, 31 May 2016 07:51:21 +0000 (07:51 +0000)
Our code is silently ignoring messages while there is an abstract method
provided for use when a message is not handled. Make sure we document
the contract and update implementations to call the method when they
encounter an unknown message.

Change-Id: I3c75fdb2c154b40c537813dd5a6bab8f47dc95cc
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/common/actor/AbstractUntypedActor.java
opendaylight/md-sal/sal-clustering-commons/src/main/java/org/opendaylight/controller/cluster/notifications/RoleChangeNotifier.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataChangeListener.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataChangeListenerRegistrationActor.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataTreeChangeListenerActor.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DataTreeChangeListenerRegistrationActor.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardReadWriteTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardWriteTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipListenerActor.java

index 50726bf..dffb610 100644 (file)
@@ -21,17 +21,24 @@ public abstract class AbstractUntypedActor extends UntypedActor {
     }
 
     @Override
-    public void onReceive(Object message) throws Exception {
+    public final void onReceive(Object message) throws Exception {
         handleReceive(message);
     }
 
+    /**
+     * Receive and handle an incoming message. If the implementation does not handle this particular message,
+     * it should call {@link #ignoreMessage(Object)} or {@link #unknownMessage(Object)}.
+     *
+     * @param message Incoming message
+     * @throws Exception
+     */
     protected abstract void handleReceive(Object message) throws Exception;
 
-    protected void ignoreMessage(Object message) {
-        LOG.debug("Unhandled message {}", message);
+    protected final void ignoreMessage(Object message) {
+        LOG.debug("Ignoring unhandled message {}", message);
     }
 
-    protected void unknownMessage(Object message) throws Exception {
+    protected final void unknownMessage(Object message) {
         LOG.debug("Received unhandled message {}", message);
         unhandled(message);
     }
index 598dfb1..1e07332 100644 (file)
@@ -45,7 +45,7 @@ public class RoleChangeNotifier extends AbstractUntypedActor implements AutoClos
     }
 
     @Override
-    protected void handleReceive(Object message) throws Exception {
+    protected void handleReceive(Object message) {
         if (message instanceof RegisterRoleChangeListener) {
             // register listeners for this shard
 
@@ -92,6 +92,8 @@ public class RoleChangeNotifier extends AbstractUntypedActor implements AutoClos
             for (ActorRef listener: registeredListeners.values()) {
                 listener.tell(latestLeaderStateChanged, getSelf());
             }
+        } else {
+            unknownMessage(message);
         }
     }
 
index cf3bdce..7f98526 100644 (file)
@@ -32,17 +32,18 @@ public class DataChangeListener extends AbstractUntypedActor {
     private final AsyncDataChangeListener<YangInstanceIdentifier, NormalizedNode<?, ?>> listener;
     private boolean notificationsEnabled = false;
 
-    public DataChangeListener(AsyncDataChangeListener<YangInstanceIdentifier,
-                                                      NormalizedNode<?, ?>> listener) {
+    public DataChangeListener(AsyncDataChangeListener<YangInstanceIdentifier, NormalizedNode<?, ?>> listener) {
         this.listener = Preconditions.checkNotNull(listener, "listener should not be null");
     }
 
     @Override
-    public void handleReceive(Object message) throws Exception {
-        if(message instanceof DataChanged){
+    public void handleReceive(Object message) {
+        if (message instanceof DataChanged){
             dataChanged(message);
-        } else if(message instanceof EnableNotification){
+        } else if (message instanceof EnableNotification){
             enableNotification((EnableNotification) message);
+        } else {
+            unknownMessage(message);
         }
     }
 
@@ -56,8 +57,7 @@ public class DataChangeListener extends AbstractUntypedActor {
 
         // Do nothing if notifications are not enabled
         if(!notificationsEnabled) {
-            LOG.debug("Notifications not enabled for listener {} - dropping change notification",
-                    listener);
+            LOG.debug("Notifications not enabled for listener {} - dropping change notification", listener);
             return;
         }
 
index b087331..2632498 100644 (file)
@@ -30,9 +30,11 @@ public class DataChangeListenerRegistrationActor extends AbstractUntypedActor {
     }
 
     @Override
-    public void handleReceive(Object message) throws Exception {
+    public void handleReceive(Object message) {
         if (message instanceof CloseDataChangeListenerRegistration) {
             closeListenerRegistration();
+        } else {
+            unknownMessage(message);
         }
     }
 
index 3f11909..03978f2 100644 (file)
@@ -37,6 +37,8 @@ final class DataTreeChangeListenerActor extends AbstractUntypedActor {
             dataChanged((DataTreeChanged)message);
         } else if (message instanceof EnableNotification) {
             enableNotification((EnableNotification) message);
+        } else {
+            unknownMessage(message);
         }
     }
 
index 7d0117f..2a106b4 100644 (file)
@@ -34,6 +34,8 @@ public final class DataTreeChangeListenerRegistrationActor extends AbstractUntyp
             registration.close();
             getSender().tell(CloseDataTreeChangeListenerRegistrationReply.getInstance(), getSelf());
             getSelf().tell(PoisonPill.getInstance(), getSelf());
+        } else {
+            unknownMessage(message);
         }
     }
 
index 8ece259..e65b8f4 100644 (file)
@@ -34,14 +34,13 @@ public class ShardReadTransaction extends ShardTransaction {
     }
 
     @Override
-    public void handleReceive(Object message) throws Exception {
+    public void handleReceive(Object message) {
         if (message instanceof CreateSnapshot) {
             createSnapshot();
         } else if(ReadData.isSerializedType(message)) {
             readData(transaction, ReadData.fromSerializable(message));
         } else if(DataExists.isSerializedType(message)) {
             dataExists(transaction, DataExists.fromSerializable(message));
-
         } else {
             super.handleReceive(message);
         }
index 515b448..2cc23c9 100644 (file)
@@ -24,7 +24,7 @@ public class ShardReadWriteTransaction extends ShardWriteTransaction {
     }
 
     @Override
-    public void handleReceive(Object message) throws Exception {
+    public void handleReceive(Object message) {
         if(ReadData.isSerializedType(message)) {
             readData(ReadData.fromSerializable(message));
         } else if(DataExists.isSerializedType(message)) {
index ebec5a1..c2a05a6 100644 (file)
@@ -16,7 +16,6 @@ import akka.japi.Creator;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import org.opendaylight.controller.cluster.common.actor.AbstractUntypedActorWithMetering;
-import org.opendaylight.controller.cluster.datastore.exceptions.UnknownMessageException;
 import org.opendaylight.controller.cluster.datastore.jmx.mbeans.shard.ShardStats;
 import org.opendaylight.controller.cluster.datastore.messages.CloseTransaction;
 import org.opendaylight.controller.cluster.datastore.messages.CloseTransactionReply;
@@ -69,16 +68,14 @@ public abstract class ShardTransaction extends AbstractUntypedActorWithMetering
     }
 
     @Override
-    public void handleReceive(Object message) throws Exception {
+    public void handleReceive(Object message) {
         if (CloseTransaction.isSerializedType(message)) {
             closeTransaction(true);
         } else if (message instanceof ReceiveTimeout) {
-            if(LOG.isDebugEnabled()) {
-                LOG.debug("Got ReceiveTimeout for inactivity - closing Tx");
-            }
+            LOG.debug("Got ReceiveTimeout for inactivity - closing transaction {}", transactionID);
             closeTransaction(false);
         } else {
-            throw new UnknownMessageException(message);
+            unknownMessage(message);
         }
     }
 
index 18294a5..2ad2488 100644 (file)
@@ -41,8 +41,7 @@ public class ShardWriteTransaction extends ShardTransaction {
     }
 
     @Override
-    public void handleReceive(Object message) throws Exception {
-
+    public void handleReceive(Object message) {
         if (message instanceof BatchedModifications) {
             batchedModifications((BatchedModifications)message);
         } else {
index 8c6b04c..32250a7 100644 (file)
@@ -32,8 +32,10 @@ class EntityOwnershipListenerActor extends AbstractUntypedActor {
 
     @Override
     protected void handleReceive(Object message) {
-        if(message instanceof EntityOwnershipChange) {
+        if (message instanceof EntityOwnershipChange) {
             onEntityOwnershipChanged((EntityOwnershipChange)message);
+        } else {
+            unknownMessage(message);
         }
     }
 

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.