You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2016/06/14 13:20:17 UTC

svn commit: r1748409 - in /jackrabbit/oak/trunk: oak-run/src/main/java/org/apache/jackrabbit/oak/explorer/ oak-run/src/main/java/org/apache/jackrabbit/oak/run/ oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/ oak-segment-tar/src/ma...

Author: mduerig
Date: Tue Jun 14 13:20:17 2016
New Revision: 1748409

URL: http://svn.apache.org/viewvc?rev=1748409&view=rev
Log:
OAK-4468: Inconsistent return values on subsequent calls to JournalReader.iterator
Make JournalReader implement Iterator instead of Iterable.

Modified:
    jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/explorer/SegmentTarExplorerBackend.java
    jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/run/SegmentTarUtils.java
    jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/JournalReader.java
    jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/TarRevisions.java
    jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java
    jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/RevisionHistory.java
    jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalEntryTest.java
    jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalReaderTest.java

Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/explorer/SegmentTarExplorerBackend.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/explorer/SegmentTarExplorerBackend.java?rev=1748409&r1=1748408&r2=1748409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/explorer/SegmentTarExplorerBackend.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/explorer/SegmentTarExplorerBackend.java Tue Jun 14 13:20:17 2016
@@ -87,7 +87,7 @@ class SegmentTarExplorerBackend implemen
             journalReader = new JournalReader(journal);
 
             try {
-                revs = newArrayList(journalReader.iterator());
+                revs = newArrayList(journalReader);
             } finally {
                 journalReader.close();
             }

Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/run/SegmentTarUtils.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/run/SegmentTarUtils.java?rev=1748409&r1=1748408&r2=1748409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/run/SegmentTarUtils.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/run/SegmentTarUtils.java Tue Jun 14 13:20:17 2016
@@ -92,6 +92,7 @@ import org.apache.jackrabbit.oak.segment
 import org.apache.jackrabbit.oak.segment.file.FileStore.ReadOnlyStore;
 import org.apache.jackrabbit.oak.segment.file.JournalReader;
 import org.apache.jackrabbit.oak.segment.file.tooling.RevisionHistory;
+import org.apache.jackrabbit.oak.segment.file.tooling.RevisionHistory.HistoryElement;
 import org.apache.jackrabbit.oak.spi.blob.BlobStore;
 import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
@@ -169,8 +170,9 @@ final class SegmentTarUtils {
     }
 
     static void history(File directory, File journal, String path, int depth) throws IOException {
-        Iterable<RevisionHistory.HistoryElement> history = new RevisionHistory(directory).getHistory(journal, path);
-        for (RevisionHistory.HistoryElement historyElement : history) {
+        Iterator<HistoryElement> history = new RevisionHistory(directory).getHistory(journal, path);
+        while (history.hasNext()) {
+            RevisionHistory.HistoryElement historyElement = history.next();
             System.out.println(historyElement.toString(depth));
         }
     }
@@ -211,7 +213,7 @@ final class SegmentTarUtils {
             File journal = new File(directory, "journal.log");
             JournalReader journalReader = new JournalReader(journal);
             try {
-                head = journalReader.iterator().next() + " root " + System.currentTimeMillis() + "\n";
+                head = journalReader.next() + " root " + System.currentTimeMillis() + "\n";
             } finally {
                 journalReader.close();
             }
@@ -275,7 +277,7 @@ final class SegmentTarUtils {
         try {
             journalReader = new JournalReader(journal);
             try {
-                revs = newArrayList(journalReader.iterator());
+                revs = newArrayList(journalReader);
             } finally {
                 journalReader.close();
             }

Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/JournalReader.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/JournalReader.java?rev=1748409&r1=1748408&r2=1748409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/JournalReader.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/JournalReader.java Tue Jun 14 13:20:17 2016
@@ -22,16 +22,16 @@ package org.apache.jackrabbit.oak.segmen
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
-import java.util.Iterator;
 
 import com.google.common.collect.AbstractIterator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Reader for journal files of the SegmentMK.
+ * Iterator over the revisions in the journal in reverse order
+ * (end of the file to beginning).
  */
-public final class JournalReader implements Closeable, Iterable<String> {
+public final class JournalReader extends AbstractIterator<String> implements Closeable {
     private static final Logger LOG = LoggerFactory.getLogger(JournalReader.class);
 
     private final ReversedLinesFileReader journal;
@@ -41,34 +41,28 @@ public final class JournalReader impleme
     }
 
     /**
-     * @return Iterator over the revisions in the journal in reverse order
-     *         (end of the file to beginning).
+     * @throws IllegalStateException  if an {@code IOException} occurs while reading from
+     *                                the journal file.
      */
     @Override
-    public Iterator<String> iterator() {
-        return new AbstractIterator<String>() {
-            @Override
-            protected String computeNext() {
-                try {
-                    String line = journal.readLine();
-                    while (line != null) {
-                        int space = line.indexOf(' ');
-                        if (space != -1) {
-                            return line.substring(0, space);
-                        }
-                        line = journal.readLine();
-                    }
-                } catch (IOException e) {
-                    LOG.error("Error reading journal file", e);
+    protected String computeNext() {
+        try {
+            String line = journal.readLine();
+            while (line != null) {
+                int space = line.indexOf(' ');
+                if (space != -1) {
+                    return line.substring(0, space);
                 }
-                return endOfData();
+                line = journal.readLine();
             }
-        };
+        } catch (IOException e) {
+            LOG.error("Error reading journal file", e);
+        }
+        return endOfData();
     }
 
     @Override
     public void close() throws IOException {
         journal.close();
     }
-
 }

Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/TarRevisions.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/TarRevisions.java?rev=1748409&r1=1748408&r2=1748409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/TarRevisions.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/TarRevisions.java Tue Jun 14 13:20:17 2016
@@ -30,7 +30,6 @@ import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.io.RandomAccessFile;
-import java.util.Iterator;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
@@ -153,9 +152,8 @@ public class TarRevisions implements Rev
         if (head.get() == null) {
             RecordId persistedId = null;
             try (JournalReader journalReader = new JournalReader(new File(directory, JOURNAL_FILE_NAME))) {
-                Iterator<String> entries = journalReader.iterator();
-                while (persistedId == null && entries.hasNext()) {
-                    String entry = entries.next();
+                while (persistedId == null && journalReader.hasNext()) {
+                    String entry = journalReader.next();
                     try {
                         RecordId id = RecordId.fromString(store, entry);
                         if (store.containsSegment(id.getSegmentId())) {

Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java?rev=1748409&r1=1748408&r2=1748409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/ConsistencyChecker.java Tue Jun 14 13:20:17 2016
@@ -30,6 +30,7 @@ import static org.apache.jackrabbit.oak.
 import static org.apache.jackrabbit.oak.segment.file.FileStoreBuilder.fileStoreBuilder;
 import static org.apache.jackrabbit.oak.spi.state.NodeStateUtils.getNode;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -53,7 +54,7 @@ import org.slf4j.LoggerFactory;
  * {@link FileStore} for inconsistency and
  * reporting that latest consistent revision.
  */
-public class ConsistencyChecker {
+public class ConsistencyChecker implements Closeable {
     private static final Logger LOG = LoggerFactory.getLogger(ConsistencyChecker.class);
 
     private final ReadOnlyStore store;
@@ -75,12 +76,13 @@ public class ConsistencyChecker {
     public static String checkConsistency(File directory, String journalFileName,
             boolean fullTraversal, long debugInterval, long binLen) throws IOException {
         print("Searching for last good revision in {}", journalFileName);
-        JournalReader journal = new JournalReader(new File(directory, journalFileName));
         Set<String> badPaths = newHashSet();
-        ConsistencyChecker checker = new ConsistencyChecker(directory, debugInterval);
-        try {
+        try (
+            JournalReader journal = new JournalReader(new File(directory, journalFileName));
+            ConsistencyChecker checker = new ConsistencyChecker(directory, debugInterval)) {
             int revisionCount = 0;
-            for (String revision : journal) {
+            while (journal.hasNext()) {
+                String revision = journal.next();
                 try {
                     print("Checking revision {}", revision);
                     revisionCount++;
@@ -100,9 +102,6 @@ public class ConsistencyChecker {
                     print("Skipping invalid record id {}", revision);
                 }
             }
-        } finally {
-            checker.close();
-            journal.close();
         }
 
         print("No good revision found");
@@ -247,6 +246,7 @@ public class ConsistencyChecker {
         return false;
     }
 
+    @Override
     public void close() {
         store.close();
     }

Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/RevisionHistory.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/RevisionHistory.java?rev=1748409&r1=1748408&r2=1748409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/RevisionHistory.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/tooling/RevisionHistory.java Tue Jun 14 13:20:17 2016
@@ -20,19 +20,20 @@
 package org.apache.jackrabbit.oak.segment.file.tooling;
 
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.collect.Iterables.transform;
 import static org.apache.jackrabbit.oak.commons.PathUtils.elements;
 import static org.apache.jackrabbit.oak.json.JsonSerializer.DEFAULT_FILTER_EXPRESSION;
 import static org.apache.jackrabbit.oak.segment.file.FileStoreBuilder.fileStoreBuilder;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Iterator;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import com.google.common.base.Function;
+import com.google.common.collect.Iterators;
 import org.apache.jackrabbit.oak.json.BlobSerializer;
 import org.apache.jackrabbit.oak.json.JsonSerializer;
 import org.apache.jackrabbit.oak.segment.SegmentNodeState;
@@ -74,10 +75,10 @@ public class RevisionHistory {
      * @return
      * @throws IOException
      */
-    public Iterable<HistoryElement> getHistory(@Nonnull File journal, @Nonnull final String path)
+    public Iterator<HistoryElement> getHistory(@Nonnull File journal, @Nonnull final String path)
             throws IOException {
         checkNotNull(path);
-        return transform(new JournalReader(checkNotNull(journal)),
+        return Iterators.transform(new JournalReader(checkNotNull(journal)),
             new Function<String, HistoryElement>() {
                 @Nullable @Override
                 public HistoryElement apply(String revision) {

Modified: jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalEntryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalEntryTest.java?rev=1748409&r1=1748408&r2=1748409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalEntryTest.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalEntryTest.java Tue Jun 14 13:20:17 2016
@@ -75,7 +75,7 @@ public class JournalEntryTest {
         assertTrue(entryTime >= startTime);
 
         JournalReader jr = new JournalReader(journal);
-        assertEquals(journalParts(lines.get(lines.size() - 1)).get(0), jr.iterator().next());
+        assertEquals(journalParts(lines.get(lines.size() - 1)).get(0), jr.next());
         jr.close();
     }
 

Modified: jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalReaderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalReaderTest.java?rev=1748409&r1=1748408&r2=1748409&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalReaderTest.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/file/JournalReaderTest.java Tue Jun 14 13:20:17 2016
@@ -26,10 +26,8 @@ import static org.junit.Assert.assertTru
 
 import java.io.File;
 import java.io.IOException;
-import java.util.Iterator;
 
-import com.google.common.collect.Iterables;
-import org.junit.Ignore;
+import com.google.common.collect.Iterators;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -41,85 +39,65 @@ public class JournalReaderTest {
 
     @Test
     public void testEmpty() throws IOException {
-        JournalReader journalReader = createJournalReader("");
-        try {
-            assertFalse(journalReader.iterator().hasNext());
-        } finally {
-            journalReader.close();
+        try (JournalReader journalReader = createJournalReader("")) {
+            assertFalse(journalReader.hasNext());
         }
     }
 
     @Test
     public void testSingleton() throws IOException {
-        JournalReader journalReader = createJournalReader("one 1");
-        try {
-            Iterator<String> journal = journalReader.iterator();
-            assertTrue(journal.hasNext());
-            assertEquals("one", journal.next());
-            assertFalse(journal.hasNext());
-        } finally {
-            journalReader.close();
+        try (JournalReader journalReader = createJournalReader("one 1")) {
+            assertTrue(journalReader.hasNext());
+            assertEquals("one", journalReader.next());
+            assertFalse(journalReader.hasNext());
         }
     }
 
     @Test
     public void testMultiple() throws IOException {
-        JournalReader journalReader = createJournalReader("one 1\ntwo 2\nthree 3 456");
-        try {
-            Iterator<String> journal = journalReader.iterator();
-            assertTrue(journal.hasNext());
-            assertEquals("three", journal.next());
-            assertTrue(journal.hasNext());
-            assertEquals("two", journal.next());
-            assertTrue(journal.hasNext());
-            assertEquals("one", journal.next());
-            assertFalse(journal.hasNext());
-        } finally {
-            journalReader.close();
+        try (JournalReader journalReader = createJournalReader("one 1\ntwo 2\nthree 3 456")) {
+            assertTrue(journalReader.hasNext());
+            assertEquals("three", journalReader.next());
+            assertTrue(journalReader.hasNext());
+            assertEquals("two", journalReader.next());
+            assertTrue(journalReader.hasNext());
+            assertEquals("one", journalReader.next());
+            assertFalse(journalReader.hasNext());
         }
     }
 
     @Test
     public void testSpaces() throws IOException {
-        JournalReader journalReader = createJournalReader("\n \n  \n   ");
-        try {
-            Iterator<String> journal = journalReader.iterator();
-            assertTrue(journal.hasNext());
-            assertEquals("", journal.next());
-            assertTrue(journal.hasNext());
-            assertEquals("", journal.next());
-            assertTrue(journal.hasNext());
-            assertEquals("", journal.next());
-            assertFalse(journal.hasNext());
-        } finally {
-            journalReader.close();
+        try (JournalReader journalReader = createJournalReader("\n \n  \n   ")) {
+            assertTrue(journalReader.hasNext());
+            assertEquals("", journalReader.next());
+            assertTrue(journalReader.hasNext());
+            assertEquals("", journalReader.next());
+            assertTrue(journalReader.hasNext());
+            assertEquals("", journalReader.next());
+            assertFalse(journalReader.hasNext());
         }
     }
 
     @Test
     public void testIgnoreInvalid() throws IOException {
-        JournalReader journalReader = createJournalReader("one 1\ntwo 2\ninvalid\nthree 3");
-        try {
-            Iterator<String> journal = journalReader.iterator();
-            assertTrue(journal.hasNext());
-            assertEquals("three", journal.next());
-            assertTrue(journal.hasNext());
-            assertEquals("two", journal.next());
-            assertTrue(journal.hasNext());
-            assertEquals("one", journal.next());
-            assertFalse(journal.hasNext());
-        } finally {
-            journalReader.close();
+        try (JournalReader journalReader = createJournalReader("one 1\ntwo 2\ninvalid\nthree 3")) {
+            assertTrue(journalReader.hasNext());
+            assertEquals("three", journalReader.next());
+            assertTrue(journalReader.hasNext());
+            assertEquals("two", journalReader.next());
+            assertTrue(journalReader.hasNext());
+            assertEquals("one", journalReader.next());
+            assertFalse(journalReader.hasNext());
         }
     }
 
-    @Ignore
     @Test
     public void testIterable() throws IOException {
         try (JournalReader journalReader = createJournalReader("one 1\ntwo 2\ninvalid\nthree 3")) {
-            assertTrue(Iterables.contains(journalReader, "one"));
-            assertTrue(Iterables.contains(journalReader, "two"));
-            assertTrue(Iterables.contains(journalReader, "three"));
+            assertTrue(Iterators.contains(journalReader, "three"));
+            assertTrue(Iterators.contains(journalReader, "two"));
+            assertTrue(Iterators.contains(journalReader, "one"));
         }
     }