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 50726bff2e618e310fbfd035fa9193a6844ae479..dffb6101c53d84ac621421c5fe4021fd640630dd 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 598dfb1fe827fe8b03234003dc5d0bdf7c750223..1e07332e514116a970c99439eb2558474d8b296a 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 cf3bdce331b21d5a3e2b9257cb809f3d87948567..7f985263c371c8dcfc669251c43dd705b2466704 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 b087331287f6f8ccf2f4e7239f4cfbbc39e74f5a..2632498a03a9db3a1f6fd43573e3eb3ce15713f1 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 3f1190911789b1f6133054b985ba5035ba77ddd4..03978f2b66387bcd483b05bcc58c4afb9ac7db8b 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 7d0117f8e18083d75e0dc20b304561919ad23509..2a106b4ddaa098368ac0cbe5d325ffe7af807588 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 8ece2593f53f4e191653fa3b93094d6e4cf091cf..e65b8f415e587d2d25dcdc50f28b8edeea66abc4 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 515b448f7b1ebf7f3f4981487c14f980a8eb4268..2cc23c90e83399629bf2045259d5d25ec1c0a972 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 ebec5a101491690f891605fd7908a1dac7aac0c3..c2a05a6d6130a3d3b28dd9879eebef70dcaf9e65 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 18294a50fa454457aa4535444a2fd7a2eb0140da..2ad2488d6d652263c07327983c97e6b429a9c625 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 8c6b04cbf3c1eeeaccf181dfc0f9a612685a8123..32250a731444fe9272acad69574e0dd09b0e6a23 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);
         }
     }