You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2018/12/01 00:29:55 UTC

[GitHub] sijie closed pull request #1836: LedgerMetadata should require digest and password together

sijie closed pull request #1836: LedgerMetadata should require digest and password together
URL: https://github.com/apache/bookkeeper/pull/1836
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 631761285a..fa32f67b33 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 @@
                    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 @@
             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 @@
             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 boolean hasPassword() {
 
     @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 88f5089300..c5a329cc33 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 @@
 
     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 static LedgerMetadataBuilder from(LedgerMetadata other) {
 
         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 LedgerMetadataBuilder withPassword(byte[] password) {
     }
 
     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 82c1a4b0bb..d9c5d35e2d 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.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 @@
     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 252c1b9d58..527ce8d39c 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 void setup() throws Exception {
         }
         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 f593642778..0316b7f269 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 @@ protected LedgerMetadata generateLedgerMetadata(int ensembleSize,
             .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 73f8d39e10..03e13585e8 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 @@ void createLedger(LedgerManager lm, Long ledgerId) throws Exception {
                 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 9db91887a6..ef57d6bddb 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 void testTriggerAuditorWithNoPendingAuditTask() throws Exception {
 
             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 95da4ffe77..a7df94f14c 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 @@ private void createNumLedgers(int numLedgers) throws Exception {
         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 void testRegisterLedgerMetadataListener() throws Exception {
 
         // 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());


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services