BlockFileLoader: correct Iterable implementation

To be iterable, `iterator()` must be able to be called twice.

This is a breaking change, as users will no longer be able to use
`BlockFileLoader` directly as an `Iterator` and must call
`.iterator()` first. See the change in `BitcoindComparisonTool`.

Also adds tests.

core/src/main/java/org/bitcoinj/utils/BlockFileLoader.java

@@ -41,7 +41,7 @@ import static org.bitcoinj.base.internal.Preconditions.checkArgument;
  * blocks together. Importing block data with this tool can be a lot faster than syncing over the network, if you
  * have the files available.</p>
  * 
- * <p>In order to comply with {@link Iterator}, this class swallows a lot of {@link IOException}s, which may result in a few
+ * <p>In order to comply with {@link Iterable}, {@link BlockIterator} swallows a lot of {@link IOException}s, which may result in a few
  * blocks being missed followed by a huge set of orphan blocks.</p>
  * 
  * <p>To blindly import all files which can be found in Bitcoin Core (version 0.8 or higher) datadir automatically,
@@ -53,7 +53,7 @@ import static org.bitcoinj.base.internal.Preconditions.checkArgument;
  * }
  * }</p>
  */
-public class BlockFileLoader implements Iterable<Block>, Iterator<Block> {
+public class BlockFileLoader implements Iterable<Block> {
     /**
      * Gets the list of files which contain blocks from Bitcoin Core.
      */
@@ -81,10 +81,7 @@ public class BlockFileLoader implements Iterable<Block>, Iterator<Block> {
         return defaultBlocksDir;
     }
 
-    private final Iterator<File> fileIt;
+    private final List<File> files;
-    private File file = null;
-    private FileInputStream currentFileStream = null;
-    private Block nextBlock = null;
     private final long packetMagic;
     private final MessageSerializer serializer;
 
@@ -93,7 +90,7 @@ public class BlockFileLoader implements Iterable<Block>, Iterator<Block> {
     }
 
     public BlockFileLoader(Network network, List<File> files) {
-        fileIt = files.iterator();
+        this.files = files;
         NetworkParameters params = NetworkParameters.of(network);
         packetMagic = params.getPacketMagic();
         serializer = params.getDefaultSerializer();
@@ -106,101 +103,113 @@ public class BlockFileLoader implements Iterable<Block>, Iterator<Block> {
 
     @Deprecated
     public BlockFileLoader(NetworkParameters params, List<File> files) {
-        fileIt = files.iterator();
+        this.files = files;
         packetMagic = params.getPacketMagic();
         serializer = params.getDefaultSerializer();
     }
 
-    @Override
-    public boolean hasNext() {
-        if (nextBlock == null)
-            loadNextBlock();
-        return nextBlock != null;
-    }
 
-    @Override
+    public class BlockIterator implements Iterator<Block>  {
-    public Block next() throws NoSuchElementException {
+        private final Iterator<File> fileIt;
-        if (!hasNext())
+        private File file = null;
-            throw new NoSuchElementException();
+        private FileInputStream currentFileStream = null;
-        Block next = nextBlock;
+        private Block nextBlock = null;
-        nextBlock = null;
-        return next;
-    }
 
-    private void loadNextBlock() {
+        public BlockIterator(List<File> fileList) {
-        while (true) {
+            this.fileIt = fileList.iterator();
-            try {
+        }
-                if (!fileIt.hasNext() && (currentFileStream == null || currentFileStream.available() < 1))
+
-                    break;
+        @Override
-            } catch (IOException e) {
+        public boolean hasNext() {
-                currentFileStream = null;
+            if (nextBlock == null)
-                if (!fileIt.hasNext())
+                loadNextBlock();
-                    break;
+            return nextBlock != null;
-            }
+        }
+
+        @Override
+        public Block next() throws NoSuchElementException {
+            if (!hasNext())
+                throw new NoSuchElementException();
+            Block next = nextBlock;
+            nextBlock = null;
+            return next;
+        }
+
+        private void loadNextBlock() {
             while (true) {
                 try {
-                    if (currentFileStream != null && currentFileStream.available() > 0)
+                    if (!fileIt.hasNext() && (currentFileStream == null || currentFileStream.available() < 1))
                         break;
-                } catch (IOException e1) {
+                } catch (IOException e) {
                     currentFileStream = null;
+                    if (!fileIt.hasNext())
+                        break;
                 }
-                if (!fileIt.hasNext()) {
+                while (true) {
-                    nextBlock = null;
+                    try {
-                    currentFileStream = null;
+                        if (currentFileStream != null && currentFileStream.available() > 0)
-                    return;
+                            break;
-                }
+                    } catch (IOException e1) {
-                file = fileIt.next();
+                        currentFileStream = null;
-                try {
+                    }
-                    currentFileStream = new FileInputStream(file);
+                    if (!fileIt.hasNext()) {
-                } catch (FileNotFoundException e) {
+                        nextBlock = null;
-                    currentFileStream = null;
+                        currentFileStream = null;
-                }
+                        return;
-            }
+                    }
-            try {
+                    file = fileIt.next();
-                int nextChar = currentFileStream.read();
+                    try {
-                while (nextChar != -1) {
+                        currentFileStream = new FileInputStream(file);
-                    if (nextChar != ((packetMagic >>> 24) & 0xff)) {
+                    } catch (FileNotFoundException e) {
-                        nextChar = currentFileStream.read();
+                        currentFileStream = null;
-                        continue;
                     }
-                    nextChar = currentFileStream.read();
-                    if (nextChar != ((packetMagic >>> 16) & 0xff))
-                        continue;
-                    nextChar = currentFileStream.read();
-                    if (nextChar != ((packetMagic >>> 8) & 0xff))
-                        continue;
-                    nextChar = currentFileStream.read();
-                    if (nextChar == (packetMagic & 0xff))
-                        break;
                 }
-                byte[] bytes = new byte[4];
-                currentFileStream.read(bytes, 0, 4);
-                long size = ByteUtils.readUint32(bytes, 0);
-                bytes = new byte[(int) size];
-                currentFileStream.read(bytes, 0, (int) size);
                 try {
-                    nextBlock = serializer.makeBlock(ByteBuffer.wrap(bytes));
+                    int nextChar = currentFileStream.read();
-                } catch (ProtocolException e) {
+                    while (nextChar != -1) {
-                    nextBlock = null;
+                        if (nextChar != ((packetMagic >>> 24) & 0xff)) {
+                            nextChar = currentFileStream.read();
+                            continue;
+                        }
+                        nextChar = currentFileStream.read();
+                        if (nextChar != ((packetMagic >>> 16) & 0xff))
+                            continue;
+                        nextChar = currentFileStream.read();
+                        if (nextChar != ((packetMagic >>> 8) & 0xff))
+                            continue;
+                        nextChar = currentFileStream.read();
+                        if (nextChar == (packetMagic & 0xff))
+                            break;
+                    }
+                    byte[] bytes = new byte[4];
+                    currentFileStream.read(bytes, 0, 4);
+                    long size = ByteUtils.readUint32(bytes, 0);
+                    bytes = new byte[(int) size];
+                    currentFileStream.read(bytes, 0, (int) size);
+                    try {
+                        nextBlock = serializer.makeBlock(ByteBuffer.wrap(bytes));
+                    } catch (ProtocolException e) {
+                        nextBlock = null;
+                        continue;
+                    } catch (Exception e) {
+                        throw new RuntimeException("unexpected problem with block in " + file, e);
+                    }
+                    break;
+                } catch (IOException e) {
+                    currentFileStream = null;
                     continue;
-                } catch (Exception e) {
-                    throw new RuntimeException("unexpected problem with block in " + file, e);
                 }
-                break;
-            } catch (IOException e) {
-                currentFileStream = null;
-                continue;
             }
         }
+
+        @Override
+        public void remove() throws UnsupportedOperationException {
+            throw new UnsupportedOperationException();
+        }
     }
 
-    @Override
-    public void remove() throws UnsupportedOperationException {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     public Iterator<Block> iterator() {
-        return this;
+        return new BlockIterator(files);
     }
 }

core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java

@@ -78,7 +78,7 @@ public class BitcoindComparisonTool {
         FullBlockTestGenerator generator = new FullBlockTestGenerator(PARAMS);
         final RuleList blockList = generator.getBlocksToTest(false, runExpensiveTests, blockFile);
         final Map<Sha256Hash, Block> preloadedBlocks = new HashMap<>();
-        final Iterator<Block> blocks = new BlockFileLoader(PARAMS.network(), Arrays.asList(blockFile));
+        final Iterator<Block> blocks = new BlockFileLoader(PARAMS.network(), Arrays.asList(blockFile)).iterator();
 
         try {
             FullPrunedBlockStore store = new MemoryFullPrunedBlockStore(PARAMS, blockList.maximumReorgBlockCount);

core/src/test/java/org/bitcoinj/utils/BlockFileLoaderTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright by the original author or authors.
+ *
+ * Licensed 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.bitcoinj.utils;
+
+import org.bitcoinj.base.BitcoinNetwork;
+import org.bitcoinj.core.Block;
+import org.bitcoinj.core.Context;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.File;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Objects;
+
+import static org.junit.Assert.assertEquals;
+
+public class BlockFileLoaderTest {
+    @Before
+    public void setUp() throws Exception {
+        Context.propagate(new Context());
+    }
+
+    @Test
+    public void iterateFirst100kCount() {
+        File blockFile = new File(getClass().getResource("../core/first-100k-blocks.dat").getFile());
+        BlockFileLoader loader = new BlockFileLoader(BitcoinNetwork.MAINNET, Collections.singletonList(blockFile));
+
+        long blockCount = 0;
+        for (Block b : loader) {
+            blockCount++;
+        }
+
+        assertEquals(439, blockCount);
+    }
+
+    @Test
+    public void iterateFirst100kTwice() {
+        File blockFile = new File(getClass().getResource("../core/first-100k-blocks.dat").getFile());
+        BlockFileLoader loader = new BlockFileLoader(BitcoinNetwork.MAINNET, Collections.singletonList(blockFile));
+
+        long blockCount = 0;
+        for (Block b : loader) {
+            blockCount++;
+        }
+
+        assertEquals(439, blockCount);
+
+        long blockCount2 = 0;
+        for (Block b : loader) {
+            blockCount2++;
+        }
+
+        assertEquals(439, blockCount2);
+    }
+}