You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/12/01 00:29:57 UTC
[bookkeeper] branch master updated: LedgerMetadata should require
digest and password together
This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 4f8fae5 LedgerMetadata should require digest and password together
4f8fae5 is described below
commit 4f8fae51802c0f51daaa6410fa929d671b58fbf5
Author: Ivan Kelly <iv...@apache.org>
AuthorDate: Sat Dec 1 01:29:53 2018 +0100
LedgerMetadata should require digest and password together
There's never a case when setting the digest and password where you
only have one, but not the other.
The patch adds validation in LedgerMetadata that if you provide one, you
provide the other. The only case where you don't provide these is when
using pre-4.1.0 (i think) metadata.
Master issue: #281
Reviewers: Sijie Guo <si...@apache.org>, Enrico Olivelli <eo...@gmail.com>
This closes #1836 from ivankelly/builder-pw
---
.../java/org/apache/bookkeeper/client/LedgerMetadata.java | 15 +++++++++++----
.../apache/bookkeeper/client/LedgerMetadataBuilder.java | 6 +++---
.../org/apache/bookkeeper/client/LedgerRecovery2Test.java | 3 ++-
.../client/ReadLastConfirmedAndEntryOpTest.java | 2 +-
.../bookkeeper/client/api/BookKeeperBuildersTest.java | 2 +-
.../apache/bookkeeper/meta/LedgerManagerIteratorTest.java | 2 +-
.../bookkeeper/replication/AuditorLedgerCheckerTest.java | 2 +-
.../bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java | 10 +++++-----
8 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
index 6317612..fa32f67 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadata.java
@@ -81,7 +81,7 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
Optional<Long> lastEntryId,
Optional<Long> length,
Map<Long, List<BookieSocketAddress>> ensembles,
- DigestType digestType,
+ Optional<DigestType> digestType,
Optional<byte[]> password,
long ctime,
boolean storeCtime,
@@ -94,6 +94,9 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
checkArgument(!length.isPresent(), "Non-closed ledger must not have a length");
checkArgument(!lastEntryId.isPresent(), "Non-closed ledger must not have a last entry");
}
+ checkArgument((digestType.isPresent() && password.isPresent())
+ || (!digestType.isPresent() && !password.isPresent()),
+ "Either both password and digest type must be set, or neither");
this.metadataFormatVersion = metadataFormatVersion;
this.ensembleSize = ensembleSize;
@@ -116,14 +119,14 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
currentEnsemble = null;
}
- this.digestType = digestType;
-
if (password.isPresent()) {
this.password = password.get();
+ this.digestType = digestType.get();
this.hasPassword = true;
} else {
this.password = null;
this.hasPassword = false;
+ this.digestType = null;
}
this.ctime = ctime;
this.storeCtime = storeCtime;
@@ -177,7 +180,11 @@ public class LedgerMetadata implements org.apache.bookkeeper.client.api.LedgerMe
@Override
public DigestType getDigestType() {
- return digestType;
+ if (!hasPassword()) {
+ return null;
+ } else {
+ return digestType;
+ }
}
@Override
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java
index 88f5089..c5a329c 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java
@@ -55,7 +55,7 @@ public class LedgerMetadataBuilder {
private TreeMap<Long, List<BookieSocketAddress>> ensembles = new TreeMap<>();
- private DigestType digestType = DigestType.CRC32C;
+ private Optional<DigestType> digestType = Optional.empty();
private Optional<byte[]> password = Optional.empty();
private long ctime = -1;
@@ -81,9 +81,9 @@ public class LedgerMetadataBuilder {
builder.ensembles.putAll(other.getAllEnsembles());
- builder.digestType = other.getDigestType();
if (other.hasPassword()) {
builder.password = Optional.of(other.getPassword());
+ builder.digestType = Optional.of(other.getDigestType());
}
builder.ctime = other.getCtime();
@@ -105,7 +105,7 @@ public class LedgerMetadataBuilder {
}
public LedgerMetadataBuilder withDigestType(DigestType digestType) {
- this.digestType = digestType;
+ this.digestType = Optional.of(digestType);
return this;
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecovery2Test.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecovery2Test.java
index 82c1a4b..d9c5d35 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecovery2Test.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerRecovery2Test.java
@@ -25,6 +25,7 @@ import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
+import org.apache.bookkeeper.client.api.DigestType;
import org.apache.bookkeeper.common.concurrent.FutureUtils;
import org.apache.bookkeeper.net.BookieSocketAddress;
import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.GenericCallbackFuture;
@@ -51,7 +52,7 @@ public class LedgerRecovery2Test {
private static Versioned<LedgerMetadata> setupLedger(ClientContext clientCtx, long ledgerId,
List<BookieSocketAddress> bookies) throws Exception {
LedgerMetadata md = LedgerMetadataBuilder.create()
- .withPassword(PASSWD)
+ .withPassword(PASSWD).withDigestType(DigestType.CRC32C)
.newEnsembleEntry(0, bookies).build();
return clientCtx.getLedgerManager().createLedgerMetadata(1L, md).get();
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOpTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOpTest.java
index 252c1b9..527ce8d 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOpTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOpTest.java
@@ -105,8 +105,8 @@ public class ReadLastConfirmedAndEntryOpTest {
}
this.ledgerMetadata = LedgerMetadataBuilder.create()
.withEnsembleSize(3).withWriteQuorumSize(2).withAckQuorumSize(2)
- .withDigestType(DigestType.CRC32.toApiDigestType())
.withPassword(new byte[0])
+ .withDigestType(DigestType.CRC32.toApiDigestType())
.newEnsembleEntry(0L, ensemble).build();
this.distributionSchedule = new RoundRobinDistributionSchedule(3, 2, 3);
// schedulers
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperBuildersTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperBuildersTest.java
index f593642..0316b7f 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperBuildersTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/api/BookKeeperBuildersTest.java
@@ -410,8 +410,8 @@ public class BookKeeperBuildersTest extends MockBookKeeperTestCase {
.withEnsembleSize(ensembleSize)
.withWriteQuorumSize(writeQuorumSize)
.withAckQuorumSize(ackQuorumSize)
- .withDigestType(BookKeeper.DigestType.CRC32.toApiDigestType())
.withPassword(password)
+ .withDigestType(BookKeeper.DigestType.CRC32.toApiDigestType())
.withCustomMetadata(customMetadata)
.withCreationTime(System.currentTimeMillis())
.newEnsembleEntry(0, generateNewEnsemble(ensembleSize)).build();
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerIteratorTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerIteratorTest.java
index 73f8d39..03e1358 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerIteratorTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/LedgerManagerIteratorTest.java
@@ -93,8 +93,8 @@ public class LedgerManagerIteratorTest extends LedgerManagerTestCase {
new BookieSocketAddress("192.0.2.3", 1234));
LedgerMetadata meta = LedgerMetadataBuilder.create()
.withEnsembleSize(3).withWriteQuorumSize(3).withAckQuorumSize(2)
- .withDigestType(BookKeeper.DigestType.CRC32.toApiDigestType())
.withPassword("passwd".getBytes())
+ .withDigestType(BookKeeper.DigestType.CRC32.toApiDigestType())
.newEnsembleEntry(0L, ensemble)
.build();
lm.createLedgerMetadata(ledgerId, meta).get();
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorLedgerCheckerTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorLedgerCheckerTest.java
index 9db9188..ef57d6b 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorLedgerCheckerTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorLedgerCheckerTest.java
@@ -553,8 +553,8 @@ public class AuditorLedgerCheckerTest extends BookKeeperClusterTestCase {
LedgerMetadata metadata = LedgerMetadataBuilder.create()
.withEnsembleSize(3).withWriteQuorumSize(2).withAckQuorumSize(2)
- .withDigestType(DigestType.CRC32.toApiDigestType())
.withPassword("passwd".getBytes())
+ .withDigestType(DigestType.CRC32.toApiDigestType())
.newEnsembleEntry(0L, ensemble).build();
long ledgerId = (Math.abs(rand.nextLong())) % 100000000;
diff --git a/metadata-drivers/etcd/src/test/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java b/metadata-drivers/etcd/src/test/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java
index 95da4ff..a7df94f 100644
--- a/metadata-drivers/etcd/src/test/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java
+++ b/metadata-drivers/etcd/src/test/java/org/apache/bookkeeper/metadata/etcd/EtcdLedgerManagerTest.java
@@ -241,8 +241,8 @@ public class EtcdLedgerManagerTest extends EtcdTestBase {
for (int i = 0; i < numLedgers; i++) {
LedgerMetadata metadata = LedgerMetadataBuilder.create()
.withEnsembleSize(3).withWriteQuorumSize(3).withAckQuorumSize(2)
- .withDigestType(DigestType.CRC32C.toApiDigestType())
.withPassword("test-password".getBytes(UTF_8))
+ .withDigestType(DigestType.CRC32C.toApiDigestType())
.newEnsembleEntry(0L, createNumBookies(3)).build();
createFutures.add(lm.createLedgerMetadata(i, metadata));
}
@@ -255,10 +255,10 @@ public class EtcdLedgerManagerTest extends EtcdTestBase {
// create a ledger metadata
LedgerMetadata metadata = LedgerMetadataBuilder.create()
- .withEnsembleSize(3).withWriteQuorumSize(3).withAckQuorumSize(2)
- .withDigestType(DigestType.CRC32C.toApiDigestType())
- .withPassword("test-password".getBytes(UTF_8))
- .newEnsembleEntry(0L, createNumBookies(3)).build();
+ .withEnsembleSize(3).withWriteQuorumSize(3).withAckQuorumSize(2)
+ .withPassword("test-password".getBytes(UTF_8))
+ .withDigestType(DigestType.CRC32C.toApiDigestType())
+ .newEnsembleEntry(0L, createNumBookies(3)).build();
result(lm.createLedgerMetadata(ledgerId, metadata));
Versioned<LedgerMetadata> readMetadata = lm.readLedgerMetadata(ledgerId).get();
log.info("Create ledger metadata : {}", readMetadata.getValue());