LUCENE-5223: Fixed IndexUpgrader command line parsing: -verbose is not required and -dir-impl option now works correctly

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1524521 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -71,6 +71,9 @@ Bug Fixes
 * LUCENE-4998: Fixed a few places to pass IOContext.READONCE instead
   of IOContext.READ (Shikhar Bhushan via Mike McCandless)
 
+* LUCENE-5223: Fixed IndexUpgrader command line parsing: -verbose is not required
+  and -dir-impl option now works correctly.  (hossman)
+
 Changes in backwards compatibility policy
 
 * LUCENE-5204: Directory doesn't have default implementations for

lucene/core/src/java/org/apache/lucene/index/IndexUpgrader.java

@@ -71,6 +71,9 @@ public final class IndexUpgrader {
    *  command-line. */
   @SuppressWarnings("deprecation")
   public static void main(String[] args) throws IOException {
+    parseArgs(args).upgrade();
+  }
+  static IndexUpgrader parseArgs(String[] args) throws IOException {
     String path = null;
     boolean deletePriorCommits = false;
     PrintStream out = null;
@@ -82,8 +85,6 @@ public final class IndexUpgrader {
         deletePriorCommits = true;
       } else if ("-verbose".equals(arg)) {
         out = System.out;
-      } else if (path == null) {
-        path = arg;
       } else if ("-dir-impl".equals(arg)) {
         if (i == args.length - 1) {
           System.out.println("ERROR: missing value for -dir-impl option");
@@ -91,6 +92,8 @@ public final class IndexUpgrader {
         }
         i++;
         dirImpl = args[i];
+      } else if (path == null) {
+        path = arg;
       }else {
         printUsage();
       }
@@ -106,7 +109,7 @@ public final class IndexUpgrader {
     } else {
       dir = CommandLineUtil.newFSDirectory(dirImpl, new File(path));
     }
-    new IndexUpgrader(dir, Version.LUCENE_CURRENT, out, deletePriorCommits).upgrade();
+    return new IndexUpgrader(dir, Version.LUCENE_CURRENT, out, deletePriorCommits);
   }
   
   private final Directory dir;
@@ -123,7 +126,10 @@ public final class IndexUpgrader {
    * {@code matchVersion}. You have the possibility to upgrade indexes with multiple commit points by removing
    * all older ones. If {@code infoStream} is not {@code null}, all logging output will be sent to this stream. */
   public IndexUpgrader(Directory dir, Version matchVersion, PrintStream infoStream, boolean deletePriorCommits) {
-    this(dir, new IndexWriterConfig(matchVersion, null).setInfoStream(infoStream), deletePriorCommits);
+    this(dir, new IndexWriterConfig(matchVersion, null), deletePriorCommits);
+    if (null != infoStream) {
+      this.iwc.setInfoStream(infoStream);
+    }
   }
   
   /** Creates index upgrader on the given directory, using an {@link IndexWriter} using the given

lucene/core/src/java/org/apache/lucene/index/IndexWriterConfig.java

@@ -497,9 +497,11 @@ public final class IndexWriterConfig extends LiveIndexWriterConfig implements Cl
     return super.getRAMBufferSizeMB();
   }
   
-  /** If non-null, information about merges, deletes and a
+  /** 
+   * Information about merges, deletes and a
    * message when maxFieldLength is reached will be printed
-   * to this.
+   * to this. Must not be null, but {@link InfoStream#NO_OUTPUT} 
+   * may be used to supress output.
    */
   public IndexWriterConfig setInfoStream(InfoStream infoStream) {
     if (infoStream == null) {
@@ -510,7 +512,9 @@ public final class IndexWriterConfig extends LiveIndexWriterConfig implements Cl
     return this;
   }
   
-  /** Convenience method that uses {@link PrintStreamInfoStream} */
+  /** 
+   * Convenience method that uses {@link PrintStreamInfoStream}.  Must not be null.
+   */
   public IndexWriterConfig setInfoStream(PrintStream printStream) {
     if (printStream == null) {
       throw new IllegalArgumentException("printStream must not be null");

lucene/core/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java

@@ -53,6 +53,9 @@ import org.apache.lucene.search.ScoreDoc;
 import org.apache.lucene.search.TermQuery;
 import org.apache.lucene.store.BaseDirectoryWrapper;
 import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.FSDirectory;
+import org.apache.lucene.store.SimpleFSDirectory;
+import org.apache.lucene.store.NIOFSDirectory;
 import org.apache.lucene.store.RAMDirectory;
 import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.BytesRef;
@@ -193,6 +196,22 @@ public class TestBackwardsCompatibility extends LuceneTestCase {
   
   static Map<String,Directory> oldIndexDirs;
 
+  /**
+   * Randomizes the use of some of hte constructor variations
+   */
+  private static IndexUpgrader newIndexUpgrader(Directory dir) {
+    final boolean streamType = random().nextBoolean();
+    final int choice = _TestUtil.nextInt(random(), 0, 2);
+    switch (choice) {
+      case 0: return new IndexUpgrader(dir, TEST_VERSION_CURRENT);
+      case 1: return new IndexUpgrader(dir, TEST_VERSION_CURRENT, 
+                                       streamType ? null : System.err, false);
+      case 2: return new IndexUpgrader(dir, newIndexWriterConfig(TEST_VERSION_CURRENT, null), false);
+      default: fail("case statement didn't get updated when random bounds changed");
+    }
+    return null; // never get here
+  }
+
   @BeforeClass
   public static void beforeClass() throws Exception {
     List<String> names = new ArrayList<String>(oldNames.length + oldSingleSegmentNames.length);
@@ -917,8 +936,7 @@ public class TestBackwardsCompatibility extends LuceneTestCase {
       }
       Directory dir = newDirectory(oldIndexDirs.get(name));
 
-      new IndexUpgrader(dir, newIndexWriterConfig(TEST_VERSION_CURRENT, null), false)
+      newIndexUpgrader(dir).upgrade();
-        .upgrade();
 
       checkAllSegmentsUpgraded(dir);
       
@@ -926,6 +944,52 @@ public class TestBackwardsCompatibility extends LuceneTestCase {
     }
   }
 
+  public void testCommandLineArgs() throws Exception {
+
+    for (String name : oldIndexDirs.keySet()) {
+      File dir = _TestUtil.getTempDir(name);
+      File dataFile = new File(TestBackwardsCompatibility.class.getResource("index." + name + ".zip").toURI());
+      _TestUtil.unzip(dataFile, dir);
+
+      String path = dir.getAbsolutePath();
+      
+      List<String> args = new ArrayList<String>();
+      if (random().nextBoolean()) {
+        args.add("-verbose");
+      }
+      if (random().nextBoolean()) {
+        args.add("-delete-prior-commits");
+      }
+      if (random().nextBoolean()) {
+        // TODO: need to better randomize this, but ...
+        //  - LuceneTestCase.FS_DIRECTORIES is private
+        //  - newFSDirectory returns BaseDirectoryWrapper
+        //  - BaseDirectoryWrapper doesn't expose delegate
+        Class<? extends FSDirectory> dirImpl = random().nextBoolean() ?
+          SimpleFSDirectory.class : NIOFSDirectory.class;
+
+        args.add("-dir-impl");
+        args.add(dirImpl.getName());
+      }
+      args.add(path);
+
+      IndexUpgrader upgrader = null;
+      try {
+        upgrader = IndexUpgrader.parseArgs(args.toArray(new String[0]));
+      } catch (Exception e) {
+        throw new AssertionError("unable to parse args: " + args, e);
+      }
+      upgrader.upgrade();
+      
+      Directory upgradedDir = newFSDirectory(dir);
+      try {
+        checkAllSegmentsUpgraded(upgradedDir);
+      } finally {
+        upgradedDir.close();
+      }
+    }
+  }
+
   public void testUpgradeOldSingleSegmentIndexWithAdditions() throws Exception {
     for (String name : oldSingleSegmentNames) {
       if (VERBOSE) {
@@ -963,8 +1027,7 @@ public class TestBackwardsCompatibility extends LuceneTestCase {
       // determine count of segments in modified index
       final int origSegCount = getNumberOfSegments(dir);
       
-      new IndexUpgrader(dir, newIndexWriterConfig(TEST_VERSION_CURRENT, null), false)
+      newIndexUpgrader(dir).upgrade();
-        .upgrade();
 
       final int segCount = checkAllSegmentsUpgraded(dir);
       assertEquals("Index must still contain the same number of segments, as only one segment was upgraded and nothing else merged",