Enforce non-null entries field in AppendEntries

The List<ReplicatedLogEntry> entries field has to be non-null
so enforce it with PreCondition. Same with leaderId. Callers
of getEntries do not need to check for null.

Change-Id: I5cd404d54e453e43456952ea2e11ea7f8f1c626c
Signed-off-by: Tcm Pantelis <tompantelis@gmail.com>

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
index 8aee8c1af88c31da507bede83065d3d457b76428..15877194173cf330d85f221688ab890366033be4 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
@@ -126,7 +126,7 @@ public class Follower extends AbstractRaftActorBehavior {
     @Override
     protected RaftActorBehavior handleAppendEntries(final ActorRef sender, final AppendEntries appendEntries) {
 
-        int numLogEntries = appendEntries.getEntries() != null ? appendEntries.getEntries().size() : 0;
+        int numLogEntries = appendEntries.getEntries().size();
         if (log.isTraceEnabled()) {
             log.trace("{}: handleAppendEntries: {}", logName(), appendEntries);
         } else if (log.isDebugEnabled() && numLogEntries > 0) {
@@ -174,10 +174,8 @@ public class Follower extends AbstractRaftActorBehavior {
             return this;
         }
 
-        if (appendEntries.getEntries() != null && appendEntries.getEntries().size() > 0) {
-
-            log.debug("{}: Number of entries to be appended = {}", logName(),
-                        appendEntries.getEntries().size());
+        if (numLogEntries > 0) {
+            log.debug("{}: Number of entries to be appended = {}", logName(), numLogEntries);
 
             // 3. If an existing entry conflicts with a new one (same index
             // but different terms), delete the existing entry and all that
@@ -186,7 +184,7 @@ public class Follower extends AbstractRaftActorBehavior {
             if (context.getReplicatedLog().size() > 0) {
 
                 // Find the entry up until the one that is not in the follower's log
-                for (int i = 0;i < appendEntries.getEntries().size(); i++, addEntriesFrom++) {
+                for (int i = 0;i < numLogEntries; i++, addEntriesFrom++) {
                     ReplicatedLogEntry matchEntry = appendEntries.getEntries().get(i);
 
                     if (!isLogEntryPresent(matchEntry.getIndex())) {
@@ -246,15 +244,15 @@ public class Follower extends AbstractRaftActorBehavior {
             // purged from the persisted log as well.
             final AtomicBoolean shouldCaptureSnapshot = new AtomicBoolean(false);
             final Procedure<ReplicatedLogEntry> appendAndPersistCallback = logEntry -> {
-                final ReplicatedLogEntry lastEntryToAppend = appendEntries.getEntries().get(
-                        appendEntries.getEntries().size() - 1);
+                final List<ReplicatedLogEntry> entries = appendEntries.getEntries();
+                final ReplicatedLogEntry lastEntryToAppend = entries.get(entries.size() - 1);
                 if (shouldCaptureSnapshot.get() && logEntry == lastEntryToAppend) {
                     context.getSnapshotManager().capture(context.getReplicatedLog().last(), getReplicatedToAllIndex());
                 }
             };
 
             // 4. Append any new entries not already in the log
-            for (int i = addEntriesFrom; i < appendEntries.getEntries().size(); i++) {
+            for (int i = addEntriesFrom; i < numLogEntries; i++) {
                 ReplicatedLogEntry entry = appendEntries.getEntries().get(i);
 
                 log.debug("{}: Append entry to log {}", logName(), entry.getData());
@@ -370,7 +368,7 @@ public class Follower extends AbstractRaftActorBehavior {
             }
 
             final List<ReplicatedLogEntry> entries = appendEntries.getEntries();
-            if (entries != null && entries.size() > 0 && !isLogEntryPresent(entries.get(0).getIndex() - 1)) {
+            if (entries.size() > 0 && !isLogEntryPresent(entries.get(0).getIndex() - 1)) {
                 log.info("{}: Cannot append entries because the calculated previousIndex {} was not found in the "
                         + "in-memory journal", logName(), entries.get(0).getIndex() - 1);
                 return true;

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntries.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntries.java
index 017794679c1e2deb4a5ece3c25f641cca32799ef..d878e2c9c3cab8ba6105a97650ebc9e815c70c46 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntries.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AppendEntries.java
@@ -8,12 +8,14 @@
 
 package org.opendaylight.controller.cluster.raft.messages;
 
+import com.google.common.base.Preconditions;
 import java.io.Externalizable;
 import java.io.IOException;
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
 import java.util.ArrayList;
 import java.util.List;
+import javax.annotation.Nonnull;
 import org.opendaylight.controller.cluster.raft.ReplicatedLogEntry;
 import org.opendaylight.controller.cluster.raft.persisted.SimpleReplicatedLogEntry;
 import org.opendaylight.controller.cluster.raft.protobuff.client.messages.Payload;
@@ -45,18 +47,20 @@ public class AppendEntries extends AbstractRaftRPC {
 
     private final short payloadVersion;
 
-    public AppendEntries(long term, String leaderId, long prevLogIndex, long prevLogTerm,
-            List<ReplicatedLogEntry> entries, long leaderCommit, long replicatedToAllIndex, short payloadVersion) {
+    public AppendEntries(long term, @Nonnull String leaderId, long prevLogIndex, long prevLogTerm,
+            @Nonnull List<ReplicatedLogEntry> entries, long leaderCommit, long replicatedToAllIndex,
+            short payloadVersion) {
         super(term);
-        this.leaderId = leaderId;
+        this.leaderId = Preconditions.checkNotNull(leaderId);
         this.prevLogIndex = prevLogIndex;
         this.prevLogTerm = prevLogTerm;
-        this.entries = entries;
+        this.entries = Preconditions.checkNotNull(entries);
         this.leaderCommit = leaderCommit;
         this.replicatedToAllIndex = replicatedToAllIndex;
         this.payloadVersion = payloadVersion;
     }
 
+    @Nonnull
     public String getLeaderId() {
         return leaderId;
     }
@@ -69,6 +73,7 @@ public class AppendEntries extends AbstractRaftRPC {
         return prevLogTerm;
     }
 
+    @Nonnull
     public List<ReplicatedLogEntry> getEntries() {
         return entries;
     }

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehaviorTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehaviorTest.java
index 3dba8d240fe1f4eb7f0d12967a1ce817455eae1f..03950dbd4b62e605083c708d475f7379b42915bd 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehaviorTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehaviorTest.java
@@ -20,6 +20,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.ObjectOutputStream;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import org.junit.After;
@@ -101,7 +102,8 @@ public abstract class AbstractRaftActorBehaviorTest<T extends RaftActorBehavior>
         // First set the receivers term to a high number (1000)
         context.getTermInformation().update(1000, "test");
 
-        AppendEntries appendEntries = new AppendEntries(100, "leader-1", 0, 0, null, 101, -1, (short)4);
+        AppendEntries appendEntries = new AppendEntries(100, "leader-1", 0, 0, Collections.emptyList(), 101, -1,
+                (short)4);
 
         behavior = createBehavior(context);
 
@@ -329,7 +331,7 @@ public abstract class AbstractRaftActorBehaviorTest<T extends RaftActorBehavior>
     }
 
     protected AppendEntries createAppendEntriesWithNewerTerm() {
-        return new AppendEntries(100, "leader-1", 0, 0, null, 1, -1, (short)0);
+        return new AppendEntries(100, "leader-1", 0, 0, Collections.emptyList(), 1, -1, (short)0);
     }
 
     protected AppendEntriesReply createAppendEntriesReplyWithNewerTerm() {

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java
index 15fca0cb8d80696393e30ea465d5263ba339fa91..37a8b6b9065b7177368d4e345d3dc3268ad8d59e 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/FollowerTest.java
@@ -538,7 +538,8 @@ public class FollowerTest extends AbstractRaftActorBehaviorTest<Follower> {
 
         // AppendEntries is now sent with a bigger term
         // this will set the receivers term to be the same as the sender's term
-        AppendEntries appendEntries = new AppendEntries(100, "leader", 0, 0, null, 101, -1, (short)0);
+        AppendEntries appendEntries = new AppendEntries(100, "leader", 0, 0, Collections.emptyList(), 101, -1,
+                (short)0);
 
         follower = createBehavior(context);