Skip to content

Commit

Permalink
(feat) LogEntry's data should not be null, close sofastack#562 (sofas…
Browse files Browse the repository at this point in the history
  • Loading branch information
killme2008 authored Apr 2, 2021
1 parent 6569a15 commit a825cc5
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,26 @@
*/
public class LogEntry implements Checksum {

public static final ByteBuffer EMPTY_DATA = ByteBuffer.wrap(new byte[0]);

/** entry type */
private EnumOutter.EntryType type;
private EnumOutter.EntryType type;
/** log id with index/term */
private LogId id = new LogId(0, 0);
private LogId id = new LogId(0, 0);
/** log entry current peers */
private List<PeerId> peers;
private List<PeerId> peers;
/** log entry old peers */
private List<PeerId> oldPeers;
private List<PeerId> oldPeers;
/** log entry current learners */
private List<PeerId> learners;
private List<PeerId> learners;
/** log entry old learners */
private List<PeerId> oldLearners;
private List<PeerId> oldLearners;
/** entry data */
private ByteBuffer data;
private ByteBuffer data = EMPTY_DATA;
/** checksum for log entry*/
private long checksum;
private long checksum;
/** true when the log has checksum **/
private boolean hasChecksum;
private boolean hasChecksum;

public List<PeerId> getLearners() {
return this.learners;
Expand Down
14 changes: 8 additions & 6 deletions jraft-core/src/main/java/com/alipay/sofa/jraft/entity/Task.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import com.alipay.sofa.jraft.Closure;
import com.alipay.sofa.jraft.closure.JoinableClosure;
import com.alipay.sofa.jraft.util.Requires;

/**
* Basic message structure of jraft, contains:
Expand All @@ -42,7 +43,7 @@ public class Task implements Serializable {
private static final long serialVersionUID = 2971309899898274575L;

/** Associated task data*/
private ByteBuffer data;
private ByteBuffer data = LogEntry.EMPTY_DATA;
/** task closure, called when the data is successfully committed to the raft group or failures happen.*/
private Closure done;
/** Reject this task if expectedTerm doesn't match the current term of this Node if the value is not -1, default is -1.*/
Expand All @@ -55,7 +56,7 @@ public Task() {
/**
* Creates a task with data/done.
*/
public Task(ByteBuffer data, Closure done) {
public Task(final ByteBuffer data, final Closure done) {
super();
this.data = data;
this.done = done;
Expand All @@ -64,7 +65,7 @@ public Task(ByteBuffer data, Closure done) {
/**
* Creates a task with data/done/expectedTerm.
*/
public Task(ByteBuffer data, Closure done, long expectedTerm) {
public Task(final ByteBuffer data, final Closure done, final long expectedTerm) {
super();
this.data = data;
this.done = done;
Expand All @@ -75,23 +76,24 @@ public ByteBuffer getData() {
return this.data;
}

public void setData(ByteBuffer data) {
public void setData(final ByteBuffer data) {
Requires.requireNonNull(data, "data should not be null, you can use LogEntry.EMPTY_DATA instead.");
this.data = data;
}

public Closure getDone() {
return this.done;
}

public void setDone(Closure done) {
public void setDone(final Closure done) {
this.done = done;
}

public long getExpectedTerm() {
return this.expectedTerm;
}

public void setExpectedTerm(long expectedTerm) {
public void setExpectedTerm(final long expectedTerm) {
this.expectedTerm = expectedTerm;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

public class LogEntryTest {
Expand All @@ -39,7 +40,7 @@ public void testEncodeDecodeWithoutData() {
LogEntry entry = new LogEntry(EnumOutter.EntryType.ENTRY_TYPE_NO_OP);
entry.setId(new LogId(100, 3));
entry.setPeers(Arrays.asList(new PeerId("localhost", 99, 1), new PeerId("localhost", 100, 2)));
assertNull(entry.getData());
assertSame(LogEntry.EMPTY_DATA, entry.getData());
assertNull(entry.getOldPeers());

byte[] content = entry.encode();
Expand All @@ -57,7 +58,7 @@ public void testEncodeDecodeWithoutData() {
assertEquals(2, nentry.getPeers().size());
assertEquals("localhost:99:1", nentry.getPeers().get(0).toString());
assertEquals("localhost:100:2", nentry.getPeers().get(1).toString());
assertNull(nentry.getData());
assertSame(LogEntry.EMPTY_DATA, entry.getData());
assertNull(nentry.getOldPeers());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Expand All @@ -48,6 +49,24 @@ public void setup() {

protected abstract LogEntryCodecFactory newFactory();

@Test
public void testEmptyData() {
LogEntry entry = new LogEntry(EnumOutter.EntryType.ENTRY_TYPE_NO_OP);
entry.setId(new LogId(100, 3));
entry.setPeers(Arrays.asList(new PeerId("localhost", 99, 1), new PeerId("localhost", 100, 2)));
entry.setData(ByteBuffer.allocate(0));

byte[] content = this.encoder.encode(entry);

assertNotNull(content);
assertTrue(content.length > 0);

LogEntry nentry = this.decoder.decode(content);
assertNotNull(nentry);
assertNotNull(nentry.getData());
assertEquals(0, nentry.getData().remaining());
}

@Test
public void testEncodeDecodeEmpty() {
try {
Expand All @@ -65,7 +84,7 @@ public void testEncodeDecodeWithoutData() {
LogEntry entry = new LogEntry(EnumOutter.EntryType.ENTRY_TYPE_NO_OP);
entry.setId(new LogId(100, 3));
entry.setPeers(Arrays.asList(new PeerId("localhost", 99, 1), new PeerId("localhost", 100, 2)));
assertNull(entry.getData());
assertSame(LogEntry.EMPTY_DATA, entry.getData());
assertNull(entry.getOldPeers());

byte[] content = this.encoder.encode(entry);
Expand All @@ -82,7 +101,7 @@ public void testEncodeDecodeWithoutData() {
assertEquals(2, nentry.getPeers().size());
assertEquals("localhost:99:1", nentry.getPeers().get(0).toString());
assertEquals("localhost:100:2", nentry.getPeers().get(1).toString());
assertNull(nentry.getData());
assertSame(LogEntry.EMPTY_DATA, nentry.getData());
assertNull(nentry.getOldPeers());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

public class LogEntryV2CodecFactoryTest extends BaseLogEntryCodecFactoryTest {
Expand All @@ -52,7 +53,7 @@ public void testEncodeDecodeWithLearners() {
entry.setPeers(Arrays.asList(new PeerId("localhost", 99, 1), new PeerId("localhost", 100, 2)));
List<PeerId> theLearners = createLearners("192.168.1.1:8081", "192.168.1.2:8081");
entry.setLearners(theLearners);
assertNull(entry.getData());
assertSame(entry.getData(), LogEntry.EMPTY_DATA);
assertNull(entry.getOldPeers());

byte[] content = this.encoder.encode(entry);
Expand Down Expand Up @@ -88,7 +89,7 @@ private void assertPeersAndLearners(final List<PeerId> theLearners, final LogEnt
assertEquals(2, nentry.getPeers().size());
assertEquals("localhost:99:1", nentry.getPeers().get(0).toString());
assertEquals("localhost:100:2", nentry.getPeers().get(1).toString());
assertNull(nentry.getData());
assertSame(nentry.getData(), LogEntry.EMPTY_DATA);
assertNull(nentry.getOldPeers());

assertTrue(nentry.hasLearners());
Expand Down

0 comments on commit a825cc5

Please sign in to comment.