Repository: incubator-ratis
Updated Branches:
  refs/heads/master 8055e5d6e -> 380baa9fd


RATIS-362. Add a Builder for TransactionContext. Contributed by Tsz Wo Nicholas 
Sze.


Project: http://git-wip-us.apache.org/repos/asf/incubator-ratis/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ratis/commit/380baa9f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ratis/tree/380baa9f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ratis/diff/380baa9f

Branch: refs/heads/master
Commit: 380baa9fd200af3fb09c038d21d8ab4c20fd4123
Parents: 8055e5d
Author: Shashikant Banerjee <[email protected]>
Authored: Mon Nov 5 15:35:08 2018 +0530
Committer: Shashikant Banerjee <[email protected]>
Committed: Mon Nov 5 15:35:08 2018 +0530

----------------------------------------------------------------------
 .../org/apache/ratis/util/Preconditions.java    | 11 +--
 .../filestore/FileStoreStateMachine.java        | 14 +--
 .../ratis/server/impl/RaftServerImpl.java       |  7 +-
 .../apache/ratis/server/storage/RaftLog.java    |  3 +-
 .../ratis/statemachine/TransactionContext.java  | 95 ++++++++++++++++++--
 .../statemachine/impl/BaseStateMachine.java     |  8 +-
 .../impl/TransactionContextImpl.java            | 28 +++---
 .../SimpleStateMachine4Testing.java             | 10 ++-
 .../ratis/statemachine/TestStateMachine.java    |  9 +-
 9 files changed, 134 insertions(+), 51 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java
----------------------------------------------------------------------
diff --git 
a/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java 
b/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java
index 7af2201..6ebfdc7 100644
--- a/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java
+++ b/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java
@@ -73,11 +73,12 @@ public interface Preconditions {
     }
   }
 
+  static void assertNull(Object object, Supplier<String> message) {
+    assertTrue(object == null, message);
+  }
+
   static void assertNull(Object object, String name) {
-    if (object != null) {
-      throw new IllegalStateException(
-          name + " is expected to be null but "
-              + name + " = " + object + " != null, class = " + 
object.getClass());
-    }
+    assertNull(object, () -> name + " is expected to be null but "
+        + name + " = " + object + " != null, class = " + object.getClass());
   }
 }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-examples/src/main/java/org/apache/ratis/examples/filestore/FileStoreStateMachine.java
----------------------------------------------------------------------
diff --git 
a/ratis-examples/src/main/java/org/apache/ratis/examples/filestore/FileStoreStateMachine.java
 
b/ratis-examples/src/main/java/org/apache/ratis/examples/filestore/FileStoreStateMachine.java
index 04bed04..73f6d93 100644
--- 
a/ratis-examples/src/main/java/org/apache/ratis/examples/filestore/FileStoreStateMachine.java
+++ 
b/ratis-examples/src/main/java/org/apache/ratis/examples/filestore/FileStoreStateMachine.java
@@ -31,13 +31,11 @@ import org.apache.ratis.protocol.Message;
 import org.apache.ratis.protocol.RaftClientRequest;
 import org.apache.ratis.protocol.RaftGroupId;
 import org.apache.ratis.server.RaftServer;
-import org.apache.ratis.server.impl.ServerProtoUtils;
 import org.apache.ratis.server.storage.RaftStorage;
 import org.apache.ratis.statemachine.StateMachineStorage;
 import org.apache.ratis.statemachine.TransactionContext;
 import org.apache.ratis.statemachine.impl.BaseStateMachine;
 import org.apache.ratis.statemachine.impl.SimpleStateMachineStorage;
-import org.apache.ratis.statemachine.impl.TransactionContextImpl;
 import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
 import 
org.apache.ratis.thirdparty.com.google.protobuf.InvalidProtocolBufferException;
 import org.apache.ratis.util.FileUtils;
@@ -95,17 +93,19 @@ public class FileStoreStateMachine extends BaseStateMachine 
{
   public TransactionContext startTransaction(RaftClientRequest request) throws 
IOException {
     final ByteString content = request.getMessage().getContent();
     final FileStoreRequestProto proto = 
FileStoreRequestProto.parseFrom(content);
-    final StateMachineLogEntryProto log;
+    final TransactionContext.Builder b = TransactionContext.newBuilder()
+        .setStateMachine(this)
+        .setClientRequest(request);
+
     if (proto.getRequestCase() == FileStoreRequestProto.RequestCase.WRITE) {
       final WriteRequestProto write = proto.getWrite();
       final FileStoreRequestProto newProto = FileStoreRequestProto.newBuilder()
           .setWriteHeader(write.getHeader()).build();
-      log = ServerProtoUtils.toStateMachineLogEntryProto(request, 
newProto.toByteString(), write.getData());
+      
b.setLogData(newProto.toByteString()).setStateMachineData(write.getData());
     } else {
-      log = ServerProtoUtils.toStateMachineLogEntryProto(request, content, 
null);
+      b.setLogData(content);
     }
-
-    return new TransactionContextImpl(this, request, log);
+    return b.build();
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java 
b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
index ed7b8bc..09e4550 100644
--- 
a/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
+++ 
b/ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java
@@ -32,7 +32,6 @@ import org.apache.ratis.server.storage.RaftStorageDirectory;
 import org.apache.ratis.statemachine.SnapshotInfo;
 import org.apache.ratis.statemachine.StateMachine;
 import org.apache.ratis.statemachine.TransactionContext;
-import org.apache.ratis.statemachine.impl.TransactionContextImpl;
 import 
org.apache.ratis.thirdparty.com.google.common.annotations.VisibleForTesting;
 import org.apache.ratis.util.*;
 import org.slf4j.Logger;
@@ -1102,7 +1101,11 @@ public class RaftServerImpl implements 
RaftServerProtocol, RaftServerAsynchronou
       // check whether there is a TransactionContext because we are the leader.
       TransactionContext trx = role.getLeaderState()
           .map(leader -> 
leader.getTransactionContext(next.getIndex())).orElseGet(
-              () -> new TransactionContextImpl(role.getCurrentRole(), 
stateMachine, next));
+              () -> TransactionContext.newBuilder()
+                  .setServerRole(role.getCurrentRole())
+                  .setStateMachine(stateMachine)
+                  .setLogEntry(next)
+                  .build());
 
       // Let the StateMachine inject logic for committed transactions in 
sequential order.
       trx = stateMachine.applyTransactionSerial(trx);

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLog.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLog.java 
b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLog.java
index 8b88b31..00650ca 100644
--- a/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLog.java
+++ b/ratis-server/src/main/java/org/apache/ratis/server/storage/RaftLog.java
@@ -151,7 +151,7 @@ public abstract class RaftLog implements Closeable {
       }
 
       // build the log entry after calling the StateMachine
-      final LogEntryProto e = 
ServerProtoUtils.toLogEntryProto(operation.getStateMachineLogEntry(), term, 
nextIndex);
+      final LogEntryProto e = operation.initLogEntry(term, nextIndex);
 
       int entrySize = e.getSerializedSize();
       if (entrySize > maxBufferSize) {
@@ -160,7 +160,6 @@ public abstract class RaftLog implements Closeable {
                 + maxBufferSize));
       }
       appendEntry(e);
-      operation.setLogEntry(e);
       return nextIndex;
     }
   }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-server/src/main/java/org/apache/ratis/statemachine/TransactionContext.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/main/java/org/apache/ratis/statemachine/TransactionContext.java
 
b/ratis-server/src/main/java/org/apache/ratis/statemachine/TransactionContext.java
index 0a53c9d..fa1cd61 100644
--- 
a/ratis-server/src/main/java/org/apache/ratis/statemachine/TransactionContext.java
+++ 
b/ratis-server/src/main/java/org/apache/ratis/statemachine/TransactionContext.java
@@ -18,13 +18,17 @@
 package org.apache.ratis.statemachine;
 
 import org.apache.ratis.proto.RaftProtos.LogEntryProto;
-import org.apache.ratis.proto.RaftProtos.LogEntryProto.LogEntryBodyCase;
 import org.apache.ratis.proto.RaftProtos.RaftPeerRole;
 import org.apache.ratis.proto.RaftProtos.StateMachineLogEntryProto;
 import org.apache.ratis.protocol.RaftClientRequest;
+import org.apache.ratis.server.impl.ServerProtoUtils;
+import org.apache.ratis.statemachine.impl.TransactionContextImpl;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.util.Preconditions;
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Objects;
 
 /**
  * Context for a transaction.
@@ -60,6 +64,9 @@ public interface TransactionContext {
    */
   StateMachineLogEntryProto getStateMachineLogEntry();
 
+  /** Set exception in case of failure. */
+  TransactionContext setException(Exception exception);
+
   /**
    * Returns the exception from the {@link StateMachine} or the log
    * @return the exception from the {@link StateMachine} or the log
@@ -81,13 +88,12 @@ public interface TransactionContext {
   Object getStateMachineContext();
 
   /**
-   * Set the {@link LogEntryProto} the current {@link TransactionContext} 
specific to. The log
-   * entry's body case must be {@link LogEntryBodyCase#STATEMACHINELOGENTRY}. 
The current
-   * {@link TransactionContext} log entry must be null, otherwise, a exception 
will be thrown
-   * @param logEntry target {@link LogEntryProto}
-   * @return the current {@link TransactionContext} itself
+   * Initialize {@link LogEntryProto} using the internal {@link 
StateMachineLogEntryProto}.
+   * @param term The current term.
+   * @param index The index of the log entry.
+   * @return the result {@link LogEntryProto}
    */
-  TransactionContext setLogEntry(LogEntryProto logEntry);
+  LogEntryProto initLogEntry(long term, long index);
 
   /**
    * Sets the data from the {@link StateMachine}
@@ -132,4 +138,79 @@ public interface TransactionContext {
    * @return cancelled transaction
    */
   TransactionContext cancelTransaction() throws IOException;
+
+  static Builder newBuilder() {
+    return new Builder();
+  }
+
+  class Builder {
+    private RaftPeerRole serverRole = RaftPeerRole.LEADER;
+    private StateMachine stateMachine;
+    private Object stateMachineContext;
+
+    private RaftClientRequest clientRequest;
+    private LogEntryProto logEntry;
+    private StateMachineLogEntryProto stateMachineLogEntry;
+    private ByteString logData;
+    private ByteString stateMachineData;
+
+    public Builder setServerRole(RaftPeerRole serverRole) {
+      this.serverRole = serverRole;
+      return this;
+    }
+
+    public Builder setStateMachine(StateMachine stateMachine) {
+      this.stateMachine = stateMachine;
+      return this;
+    }
+
+    public Builder setStateMachineContext(Object stateMachineContext) {
+      this.stateMachineContext = stateMachineContext;
+      return this;
+    }
+
+    public Builder setClientRequest(RaftClientRequest clientRequest) {
+      this.clientRequest = clientRequest;
+      return this;
+    }
+
+    public Builder setLogEntry(LogEntryProto logEntry) {
+      this.logEntry = logEntry;
+      return this;
+    }
+
+    public Builder setStateMachineLogEntry(StateMachineLogEntryProto 
stateMachineLogEntry) {
+      this.stateMachineLogEntry = stateMachineLogEntry;
+      return this;
+    }
+
+    public Builder setLogData(ByteString logData) {
+      this.logData = logData;
+      return this;
+    }
+
+    public Builder setStateMachineData(ByteString stateMachineData) {
+      this.stateMachineData = stateMachineData;
+      return this;
+    }
+
+    public TransactionContext build() {
+      Objects.requireNonNull(serverRole, "serverRole == null");
+      Objects.requireNonNull(stateMachine, "stateMachine == null");
+      if (clientRequest != null) {
+        Preconditions.assertTrue(serverRole == RaftPeerRole.LEADER,
+            () -> "serverRole MUST be LEADER since clientRequest != null, 
serverRole is " + serverRole);
+        Preconditions.assertNull(logEntry, () -> "logEntry MUST be null since 
clientRequest != null");
+        if (stateMachineLogEntry == null) {
+          stateMachineLogEntry = 
ServerProtoUtils.toStateMachineLogEntryProto(clientRequest, logData, 
stateMachineData);
+        }
+        return new TransactionContextImpl(stateMachine, clientRequest, 
stateMachineLogEntry, stateMachineContext);
+      } else {
+        Objects.requireNonNull(logEntry, "logEntry MUST NOT be null since 
clientRequest == null");
+        Preconditions.assertTrue(logEntry.hasStateMachineLogEntry(),
+            () -> "Unexpected logEntry: stateMachineLogEntry not found, 
logEntry=" + logEntry);
+        return new TransactionContextImpl(serverRole, stateMachine, logEntry);
+      }
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java
 
b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java
index 119432e..8a09bd4 100644
--- 
a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java
+++ 
b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/BaseStateMachine.java
@@ -170,9 +170,11 @@ public class BaseStateMachine implements StateMachine {
   }
 
   @Override
-  public TransactionContext startTransaction(RaftClientRequest request)
-      throws IOException {
-    return new TransactionContextImpl(this, request, null);
+  public TransactionContext startTransaction(RaftClientRequest request) throws 
IOException {
+    return TransactionContext.newBuilder()
+        .setStateMachine(this)
+        .setClientRequest(request)
+        .build();
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
 
b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
index 5e4936a..3a2fb12 100644
--- 
a/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
+++ 
b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/TransactionContextImpl.java
@@ -18,7 +18,6 @@
 package org.apache.ratis.statemachine.impl;
 
 import org.apache.ratis.proto.RaftProtos.LogEntryProto;
-import org.apache.ratis.proto.RaftProtos.LogEntryProto.LogEntryBodyCase;
 import org.apache.ratis.proto.RaftProtos.RaftPeerRole;
 import org.apache.ratis.proto.RaftProtos.StateMachineLogEntryProto;
 import org.apache.ratis.protocol.RaftClientRequest;
@@ -32,6 +31,8 @@ import java.util.Objects;
 
 /**
  * Implementation of {@link TransactionContext}
+ *
+ * This is a private API.  Applications should use {@link TransactionContext} 
and {@link TransactionContext.Builder}.
  */
 public class TransactionContextImpl implements TransactionContext {
   /** The role of the server when this object is created. */
@@ -71,13 +72,6 @@ public class TransactionContextImpl implements 
TransactionContext {
     this.stateMachine = stateMachine;
   }
 
-  /** The same as this(stateMachine, clientRequest, smLogEntryProto, null). */
-  public TransactionContextImpl(
-      StateMachine stateMachine, RaftClientRequest clientRequest,
-      StateMachineLogEntryProto smLogEntryProto) {
-    this(stateMachine, clientRequest, smLogEntryProto, null);
-  }
-
   /**
    * Construct a {@link TransactionContext} from a client request.
    * Used by the state machine to start a transaction
@@ -101,7 +95,7 @@ public class TransactionContextImpl implements 
TransactionContext {
    */
   public TransactionContextImpl(RaftPeerRole serverRole, StateMachine 
stateMachine, LogEntryProto logEntry) {
     this(serverRole, stateMachine);
-    setLogEntry(logEntry);
+    this.logEntry = logEntry;
     this.smLogEntryProto = logEntry.getStateMachineLogEntry();
   }
 
@@ -137,14 +131,11 @@ public class TransactionContextImpl implements 
TransactionContext {
   }
 
   @Override
-  public TransactionContext setLogEntry(LogEntryProto logEntry) {
-    Objects.requireNonNull(logEntry, "logEntry == null");
-    Preconditions.assertTrue(logEntry.getLogEntryBodyCase() == 
LogEntryBodyCase.STATEMACHINELOGENTRY,
-        () -> "LogEntryBodyCase = " + logEntry.getLogEntryBodyCase()
-            + " != " + LogEntryBodyCase.STATEMACHINELOGENTRY + ", logEntry=" + 
logEntry);
-    Preconditions.assertTrue(this.logEntry == null, "this.logEntry != null");
-    this.logEntry = logEntry;
-    return this;
+  public LogEntryProto initLogEntry(long term, long index) {
+    Preconditions.assertTrue(serverRole == RaftPeerRole.LEADER);
+    Preconditions.assertNull(logEntry, "logEntry");
+    Objects.requireNonNull(smLogEntryProto, "smLogEntryProto == null");
+    return logEntry = ServerProtoUtils.toLogEntryProto(smLogEntryProto, term, 
index);
   }
 
   @Override
@@ -158,7 +149,8 @@ public class TransactionContextImpl implements 
TransactionContext {
     return logEntry;
   }
 
-  private TransactionContext setException(IOException ioe) {
+  @Override
+  public TransactionContext setException(Exception ioe) {
     assert exception != null;
     this.exception = ioe;
     return this;

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-server/src/test/java/org/apache/ratis/statemachine/SimpleStateMachine4Testing.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/test/java/org/apache/ratis/statemachine/SimpleStateMachine4Testing.java
 
b/ratis-server/src/test/java/org/apache/ratis/statemachine/SimpleStateMachine4Testing.java
index 84be87b..d5fdf53 100644
--- 
a/ratis-server/src/test/java/org/apache/ratis/statemachine/SimpleStateMachine4Testing.java
+++ 
b/ratis-server/src/test/java/org/apache/ratis/statemachine/SimpleStateMachine4Testing.java
@@ -39,7 +39,6 @@ import org.apache.ratis.server.storage.RaftStorage;
 import org.apache.ratis.statemachine.impl.BaseStateMachine;
 import org.apache.ratis.statemachine.impl.SimpleStateMachineStorage;
 import org.apache.ratis.statemachine.impl.SingleFileSnapshotInfo;
-import org.apache.ratis.statemachine.impl.TransactionContextImpl;
 import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
 import org.apache.ratis.util.Daemon;
 import org.apache.ratis.util.JavaUtils;
@@ -303,10 +302,13 @@ public class SimpleStateMachine4Testing extends 
BaseStateMachine {
   static final ByteString STATE_MACHINE_DATA = 
ByteString.copyFromUtf8("StateMachine Data");
 
   @Override
-  public TransactionContext startTransaction(RaftClientRequest request) throws 
IOException {
+  public TransactionContext startTransaction(RaftClientRequest request) {
     blocking.await(Blocking.Type.START_TRANSACTION);
-    return new TransactionContextImpl(this, request,
-        ServerProtoUtils.toStateMachineLogEntryProto(request, null, 
STATE_MACHINE_DATA));
+    return TransactionContext.newBuilder()
+        .setStateMachine(this)
+        .setClientRequest(request)
+        .setStateMachineData(STATE_MACHINE_DATA)
+        .build();
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-ratis/blob/380baa9f/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
----------------------------------------------------------------------
diff --git 
a/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
 
b/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
index 329b02b..a4dc88a 100644
--- 
a/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
+++ 
b/ratis-server/src/test/java/org/apache/ratis/statemachine/TestStateMachine.java
@@ -34,7 +34,6 @@ import org.apache.ratis.server.impl.RaftServerImpl;
 import org.apache.ratis.server.impl.RaftServerProxy;
 import org.apache.ratis.server.impl.RaftServerTestUtil;
 import org.apache.ratis.server.simulation.MiniRaftClusterWithSimulatedRpc;
-import org.apache.ratis.statemachine.impl.TransactionContextImpl;
 import org.apache.ratis.util.LogUtils;
 import org.junit.*;
 
@@ -75,11 +74,15 @@ public class TestStateMachine extends BaseTest implements 
MiniRaftClusterWithSim
     ConcurrentLinkedQueue<Long> applied = new ConcurrentLinkedQueue<>();
 
     @Override
-    public TransactionContext startTransaction(RaftClientRequest request) 
throws IOException {
+    public TransactionContext startTransaction(RaftClientRequest request) {
       // only leader will get this call
       isLeader.set(true);
       // send the next transaction id as the "context" from SM
-      return new TransactionContextImpl(this, request, null, 
transactions.incrementAndGet());
+      return TransactionContext.newBuilder()
+          .setStateMachine(this)
+          .setClientRequest(request)
+          .setStateMachineContext(transactions.incrementAndGet())
+          .build();
     }
 
     @Override

Reply via email to