From: Robert Varga <rovarga@cisco.com>
Date: Fri, 1 Apr 2016 18:31:59 +0000 (+0200)
Subject: BUG-5626: eliminate SwitchBehaviorSupplier
X-Git-Tag: release/boron~251
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=55d229e2b83b1a48a43cc1e01c90f5be7d02faf0

BUG-5626: eliminate SwitchBehaviorSupplier

This class acts as an indirection which makes following logic hard, eliminate
it in favor of explicit code.

Change-Id: I8715f6cdaed6e653d39af6741242072b39232022
Signed-off-by: Robert Varga <rovarga@cisco.com>
---

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java
index 374b2a7e78..65c265c194 100644
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/RaftActor.java
@@ -23,7 +23,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
-import java.util.function.Supplier;
 import javax.annotation.Nonnull;
 import org.apache.commons.lang3.time.DurationFormatUtils;
 import org.opendaylight.controller.cluster.DataPersistenceProvider;
@@ -115,8 +114,6 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
 
     private final BehaviorStateHolder reusableBehaviorStateHolder = new BehaviorStateHolder();
 
-    private final SwitchBehaviorSupplier reusableSwitchBehaviorSupplier = new SwitchBehaviorSupplier();
-
     private RaftActorServerConfigurationSupport serverConfigurationSupport;
 
     private RaftActorLeadershipTransferCohort leadershipTransferInProgress;
@@ -189,18 +186,19 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
     }
 
     @VisibleForTesting
-    protected void changeCurrentBehavior(RaftActorBehavior newBehavior){
-        if(getCurrentBehavior() != null) {
+    protected void changeCurrentBehavior(RaftActorBehavior newBehavior) {
+        final RaftActorBehavior currentBehavior = getCurrentBehavior();
+        if (currentBehavior != null) {
             try {
-                getCurrentBehavior().close();
-            } catch(Exception e) {
-                LOG.warn("{}: Error closing behavior {}", persistence(), getCurrentBehavior(), e);
+                currentBehavior.close();
+            } catch (Exception e) {
+                LOG.warn("{}: Error closing behavior {}", persistence(), currentBehavior, e);
             }
         }
 
-        reusableBehaviorStateHolder.init(getCurrentBehavior());
+        reusableBehaviorStateHolder.init(currentBehavior);
         setCurrentBehavior(newBehavior);
-        handleBehaviorChange(reusableBehaviorStateHolder, getCurrentBehavior());
+        handleBehaviorChange(reusableBehaviorStateHolder, newBehavior);
     }
 
     @Override
@@ -266,7 +264,12 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
         } else if(message instanceof Runnable) {
             ((Runnable)message).run();
         } else {
-            switchBehavior(reusableSwitchBehaviorSupplier.handleMessage(getSender(), message));
+            final RaftActorBehavior currentBehavior = getCurrentBehavior();
+
+            // Processing the message may affect the state, hence we need to capture it
+            reusableBehaviorStateHolder.init(currentBehavior);
+            final RaftActorBehavior nextBehavior = currentBehavior.handleMessage(getSender(), message);
+            switchBehavior(nextBehavior);
         }
     }
 
@@ -353,7 +356,10 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
         if(!getRaftActorContext().getRaftPolicy().automaticElectionsEnabled()) {
             RaftState newState = message.getNewState();
             if( newState == RaftState.Leader || newState == RaftState.Follower) {
-                switchBehavior(reusableSwitchBehaviorSupplier.handleMessage(getSender(), message));
+                reusableBehaviorStateHolder.init(getCurrentBehavior());
+                final RaftActorBehavior nextBehavior = AbstractRaftActorBehavior.createBehavior(context,
+                    message.getNewState());
+                switchBehavior(nextBehavior);
                 getRaftActorContext().getTermInformation().updateAndPersist(message.getNewTerm(), "");
             } else {
                 LOG.warn("Switching to behavior : {} - not supported", newState);
@@ -361,12 +367,9 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
         }
     }
 
-    private void switchBehavior(Supplier<RaftActorBehavior> supplier){
-        reusableBehaviorStateHolder.init(getCurrentBehavior());
-
-        setCurrentBehavior(supplier.get());
-
-        handleBehaviorChange(reusableBehaviorStateHolder, getCurrentBehavior());
+    private void switchBehavior(final RaftActorBehavior nextBehavior) {
+        setCurrentBehavior(nextBehavior);
+        handleBehaviorChange(reusableBehaviorStateHolder, nextBehavior);
     }
 
     @VisibleForTesting
@@ -880,23 +883,4 @@ public abstract class RaftActor extends AbstractUntypedPersistentActor {
             return leaderPayloadVersion;
         }
     }
-
-    private class SwitchBehaviorSupplier implements Supplier<RaftActorBehavior> {
-        private Object message;
-        private ActorRef sender;
-
-        public SwitchBehaviorSupplier handleMessage(ActorRef sender, Object message){
-            this.sender = sender;
-            this.message = message;
-            return this;
-        }
-
-        @Override
-        public RaftActorBehavior get() {
-            if(this.message instanceof SwitchBehavior){
-                return AbstractRaftActorBehavior.createBehavior(context, ((SwitchBehavior) message).getNewState());
-            }
-            return getCurrentBehavior().handleMessage(sender, message);
-        }
-    }
 }