From fd29c6cd3e5c7b0fbfd5244f164473c72ced8f7b Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 14 Feb 2016 02:32:58 +0100 Subject: [PATCH] Clean AbstractServerChangeReply up This is an abstract base class, make it abstract and non-instantiable from outside of the world. It introduces public API, hence we cannot hide it. Also switch to using MoreObjects.ToStringHelper, as that allows us to unify toString(). Change-Id: I21bdd4d0815a50393519414449c7e7eb7179b077 Signed-off-by: Robert Varga --- .../messages/AbstractServerChangeReply.java | 28 +++++++++++-------- .../cluster/raft/messages/AddServerReply.java | 2 +- .../raft/messages/RemoveServerReply.java | 7 +---- ...ftActorServerConfigurationSupportTest.java | 12 ++++---- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AbstractServerChangeReply.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AbstractServerChangeReply.java index 49bd94e6c4..3f0cf43ac9 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AbstractServerChangeReply.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AbstractServerChangeReply.java @@ -7,38 +7,42 @@ */ package org.opendaylight.controller.cluster.raft.messages; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.MoreObjects; +import com.google.common.base.Preconditions; import java.io.Serializable; +import java.util.Optional; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * Abstract base class for a server configuration change reply. * * @author Thomas Pantelis */ -public class AbstractServerChangeReply implements Serializable { +public abstract class AbstractServerChangeReply implements Serializable { private static final long serialVersionUID = 1L; private final String leaderHint; private final ServerChangeStatus status; - public AbstractServerChangeReply(ServerChangeStatus status, String leaderHint) { - this.status = status; + AbstractServerChangeReply(final @Nonnull ServerChangeStatus status, final @Nullable String leaderHint) { + this.status = Preconditions.checkNotNull(status); this.leaderHint = leaderHint; } - public static long getSerialversionuid() { - return serialVersionUID; + @VisibleForTesting + @Nonnull public final Optional getLeaderHint() { + return Optional.ofNullable(leaderHint); } - public String getLeaderHint() { - return leaderHint; - } - - public ServerChangeStatus getStatus() { + @Nonnull public final ServerChangeStatus getStatus() { return status; } @Override - public String toString() { - return getClass().getSimpleName() + " [status=" + status + ", leaderHint=" + leaderHint + "]"; + public final String toString() { + return MoreObjects.toStringHelper(getClass()).omitNullValues() + .add("status", status).add("leaderHint", leaderHint).toString(); } } diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AddServerReply.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AddServerReply.java index 40d39f45d6..fc950aa05a 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AddServerReply.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/AddServerReply.java @@ -12,7 +12,7 @@ package org.opendaylight.controller.cluster.raft.messages; * * @author Thomas Pantelis */ -public class AddServerReply extends AbstractServerChangeReply { +public final class AddServerReply extends AbstractServerChangeReply { private static final long serialVersionUID = 1L; public AddServerReply(ServerChangeStatus status, String leaderHint) { diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/RemoveServerReply.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/RemoveServerReply.java index 36a48fc60f..fe643d8af8 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/RemoveServerReply.java +++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/messages/RemoveServerReply.java @@ -13,15 +13,10 @@ import javax.annotation.Nullable; /** * Reply to a RemoveServer message (§4.1). */ -public class RemoveServerReply extends AbstractServerChangeReply { +public final class RemoveServerReply extends AbstractServerChangeReply { private static final long serialVersionUID = 1L; public RemoveServerReply(ServerChangeStatus status, @Nullable String leaderHint) { super(status, leaderHint); } - - @Override - public String toString() { - return "RemoveServerReply{" + "status=" + getStatus() + ", leaderHint='" + getLeaderHint() + '\'' + '}'; - } } diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupportTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupportTest.java index 6d102dbede..36f6fe7b1a 100644 --- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupportTest.java +++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/RaftActorServerConfigurationSupportTest.java @@ -152,7 +152,7 @@ public class RaftActorServerConfigurationSupportTest extends AbstractActorTest { AddServerReply addServerReply = testKit.expectMsgClass(JavaTestKit.duration("5 seconds"), AddServerReply.class); assertEquals("getStatus", ServerChangeStatus.OK, addServerReply.getStatus()); - assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint()); + assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint().get()); // Verify ServerConfigurationPayload entry in leader's log @@ -235,7 +235,7 @@ public class RaftActorServerConfigurationSupportTest extends AbstractActorTest { AddServerReply addServerReply = testKit.expectMsgClass(JavaTestKit.duration("5 seconds"), AddServerReply.class); assertEquals("getStatus", ServerChangeStatus.OK, addServerReply.getStatus()); - assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint()); + assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint().get()); // Verify ServerConfigurationPayload entry in leader's log @@ -276,7 +276,7 @@ public class RaftActorServerConfigurationSupportTest extends AbstractActorTest { AddServerReply addServerReply = testKit.expectMsgClass(JavaTestKit.duration("5 seconds"), AddServerReply.class); assertEquals("getStatus", ServerChangeStatus.OK, addServerReply.getStatus()); - assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint()); + assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint().get()); // Verify ServerConfigurationPayload entry in leader's log @@ -313,7 +313,7 @@ public class RaftActorServerConfigurationSupportTest extends AbstractActorTest { addServerReply = testKit.expectMsgClass(JavaTestKit.duration("5 seconds"), AddServerReply.class); assertEquals("getStatus", ServerChangeStatus.OK, addServerReply.getStatus()); - assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint()); + assertEquals("getLeaderHint", java.util.Optional.of(LEADER_ID), addServerReply.getLeaderHint()); expectFirstMatching(leaderCollectorActor, ApplyState.class); assertEquals("Leader journal last index", 1, leaderActorContext.getReplicatedLog().lastIndex()); @@ -409,7 +409,7 @@ public class RaftActorServerConfigurationSupportTest extends AbstractActorTest { AddServerReply addServerReply = testKit.expectMsgClass(JavaTestKit.duration("5 seconds"), AddServerReply.class); assertEquals("getStatus", ServerChangeStatus.OK, addServerReply.getStatus()); - assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint()); + assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint().get()); expectFirstMatching(newFollowerCollectorActor, ApplySnapshot.class); @@ -619,7 +619,7 @@ public class RaftActorServerConfigurationSupportTest extends AbstractActorTest { // The first AddServer should succeed with OK even though consensus wasn't reached AddServerReply addServerReply = testKit.expectMsgClass(JavaTestKit.duration("5 seconds"), AddServerReply.class); assertEquals("getStatus", ServerChangeStatus.OK, addServerReply.getStatus()); - assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint()); + assertEquals("getLeaderHint", LEADER_ID, addServerReply.getLeaderHint().get()); // Verify ServerConfigurationPayload entry in leader's log verifyServerConfigurationPayloadEntry(leaderActorContext.getReplicatedLog(), votingServer(LEADER_ID), -- 2.36.6