[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2457: BP-42: New Client API - list ledgers

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #2457:
URL: https://github.com/apache/bookkeeper/pull/2457#discussion_r512097588



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerMetadataBuilder.java
##########
@@ -190,10 +197,11 @@ public LedgerMetadataBuilder withCToken(long cToken) {
     }
 
     public LedgerMetadata build() {
+        checkArgument(ledgerId > 0, "Ledger id must be set");

Review comment:
       ledgerId = 0 is a valid value AFAIK

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/ListLedgersResultBuilder.java
##########
@@ -0,0 +1,34 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.client.api;
+
+import org.apache.bookkeeper.common.annotation.InterfaceAudience.Public;
+import org.apache.bookkeeper.common.annotation.InterfaceStability.Unstable;
+
+/**
+ * Builder-style interface to list exiting ledgers.
+ *

Review comment:
       nit: remove empty line

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
##########
@@ -1473,6 +1480,119 @@ public DeleteBuilder newDeleteLedgerOp() {
         return new LedgerDeleteOp.DeleteBuilderImpl(this);
     }
 
+    private static final class SyncLedgerIterator implements LedgersIterator {
+
+        private final LedgerRangeIterator iterator;
+        private final ListLedgersResultImpl parent;
+        Iterator<Long> currentRange = null;
+
+        public SyncLedgerIterator(LedgerRangeIterator iterator, ListLedgersResultImpl parent) {
+            this.iterator = iterator;
+            this.parent = parent;
+        }
+
+        @Override
+        public boolean hasNext() throws IOException {
+            parent.checkClosed();
+            if (currentRange != null) {
+                if (currentRange.hasNext()) {
+                    return true;
+                }
+            } else if (iterator.hasNext()) {
+                return true;
+            }
+            return false;
+        }
+
+        @Override
+        public long next() throws IOException {
+            parent.checkClosed();
+            if (currentRange == null || !currentRange.hasNext()) {
+                currentRange = iterator.next().getLedgers().iterator();
+            }
+            return currentRange.next();
+        }
+    }
+
+    private static final class ListLedgersResultImpl implements ListLedgersResult {
+
+        private final LedgerRangeIterator iterator;
+        private boolean closed = false;
+        private LedgersIterator ledgersIterator;
+
+        public ListLedgersResultImpl(LedgerRangeIterator iterator) {
+            this.iterator = iterator;
+        }
+
+        void checkClosed() {
+            if (closed) {
+                throw new IllegalStateException("ListLedgersResult is closed");
+            }
+        }
+
+        private void initLedgersIterator() {
+            if (ledgersIterator != null) {
+                throw new IllegalStateException("LedgersIterator must be requested once");
+            }
+            ledgersIterator = new SyncLedgerIterator(iterator, this);
+        }
+
+        @Override
+        public LedgersIterator iterator() {
+            checkClosed();
+            initLedgersIterator();
+            return ledgersIterator;
+

Review comment:
       nit: remove empty line




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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