LUCENE-3380: fix CFS and NoSuchDirectoryException issues with FileSwitchDirectory

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

lucene/CHANGES.txt

@@ -565,6 +565,11 @@ Bug fixes
   can cause IndexWriter overriding an existing index.
   (Geoff Cooney via Simon Willnauer)
 
+* LUCENE-3380: Fixed a bug where FileSwitchDirectory's listAll() would wrongly
+  throw NoSuchDirectoryException when all files written so far have been
+  written to one directory, but the other still has not yet been created on the
+  filesystem.  (Robert Muir)
+  
 New Features
 
 * LUCENE-3290: Added FieldInvertState.numUniqueTerms 

lucene/src/java/org/apache/lucene/store/FileSwitchDirectory.java

@@ -78,12 +78,39 @@ public class FileSwitchDirectory extends Directory {
   @Override
   public String[] listAll() throws IOException {
     Set<String> files = new HashSet<String>();
+    // LUCENE-3380: either or both of our dirs could be FSDirs,
+    // but if one underlying delegate is an FSDir and mkdirs() has not
+    // yet been called, because so far everything is written to the other,
+    // in this case, we don't want to throw a NoSuchDirectoryException
+    NoSuchDirectoryException exc = null;
+    try {
       for(String f : primaryDir.listAll()) {
         files.add(f);
       }
+    } catch (NoSuchDirectoryException e) {
+      exc = e;
+    }
+    try {
       for(String f : secondaryDir.listAll()) {
         files.add(f);
       }
+    } catch (NoSuchDirectoryException e) {
+      // we got NoSuchDirectoryException from both dirs
+      // rethrow the first.
+      if (exc != null) {
+        throw exc;
+      }
+      // we got NoSuchDirectoryException from the secondary,
+      // and the primary is empty.
+      if (files.isEmpty()) {
+        throw e;
+      }
+    }
+    // we got NoSuchDirectoryException from the primary,
+    // and the secondary is empty.
+    if (exc != null && files.isEmpty()) {
+      throw exc;
+    }
     return files.toArray(new String[files.size()]);
   }
 
@@ -150,13 +177,19 @@ public class FileSwitchDirectory extends Directory {
     return getDirectory(name).openInput(name, context);
   }
 
+  // final due to LUCENE-3380: currently CFS backdoors the directory to create CFE
+  // by using the basic implementation and not delegating, we ensure that all 
+  // openInput/createOutput requests come thru NRTCachingDirectory.
   @Override
-  public CompoundFileDirectory openCompoundInput(String name, IOContext context) throws IOException {
+  public final CompoundFileDirectory openCompoundInput(String name, IOContext context) throws IOException {
-    return getDirectory(name).openCompoundInput(name, context);
+    return super.openCompoundInput(name, context);
   }
   
+  // final due to LUCENE-3380: currently CFS backdoors the directory to create CFE
+  // by using the basic implementation and not delegating, we ensure that all 
+  // openInput/createOutput requests come thru NRTCachingDirectory.
   @Override
-  public CompoundFileDirectory createCompoundOutput(String name, IOContext context) throws IOException {
+  public final CompoundFileDirectory createCompoundOutput(String name, IOContext context) throws IOException {
-    return getDirectory(name).createCompoundOutput(name, context);
+    return super.createCompoundOutput(name, context);
   }
 }

lucene/src/test/org/apache/lucene/store/TestFileSwitchDirectory.java

@@ -18,6 +18,8 @@ package org.apache.lucene.store;
  */
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -28,6 +30,7 @@ import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.IndexWriterConfig;
 import org.apache.lucene.index.TestIndexWriterReader;
 import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util._TestUtil;
 
 public class TestFileSwitchDirectory extends LuceneTestCase {
   /**
@@ -77,4 +80,76 @@ public class TestFileSwitchDirectory extends LuceneTestCase {
     }
     fsd.close();
   }
+  
+  private Directory newFSSwitchDirectory(Set<String> primaryExtensions) throws IOException {
+    Directory a = new SimpleFSDirectory(_TestUtil.getTempDir("foo"));
+    Directory b = new SimpleFSDirectory(_TestUtil.getTempDir("bar"));
+    FileSwitchDirectory switchDir = new FileSwitchDirectory(primaryExtensions, a, b, true);
+    return new MockDirectoryWrapper(random, switchDir);
+  }
+  
+  // LUCENE-3380 -- make sure we get exception if the directory really does not exist.
+  public void testNoDir() throws Throwable {
+    Directory dir = newFSSwitchDirectory(Collections.<String>emptySet());
+    try {
+      IndexReader.open(dir, true);
+      fail("did not hit expected exception");
+    } catch (NoSuchDirectoryException nsde) {
+      // expected
+    }
+    dir.close();
+  }
+  
+  // LUCENE-3380 test that we can add a file, and then when we call list() we get it back
+  public void testDirectoryFilter() throws IOException {
+    Directory dir = newFSSwitchDirectory(Collections.<String>emptySet());
+    String name = "file";
+    try {
+      dir.createOutput(name, newIOContext(random)).close();
+      assertTrue(dir.fileExists(name));
+      assertTrue(Arrays.asList(dir.listAll()).contains(name));
+    } finally {
+      dir.close();
+    }
+  }
+  
+  // LUCENE-3380 test that delegate compound files correctly.
+  public void testCompoundFileAppendTwice() throws IOException {
+    Directory newDir = newFSSwitchDirectory(Collections.singleton("cfs"));
+    CompoundFileDirectory csw = newDir.createCompoundOutput("d.cfs", newIOContext(random));
+    createSequenceFile(newDir, "d1", (byte) 0, 15);
+    IndexOutput out = csw.createOutput("d.xyz", newIOContext(random));
+    out.writeInt(0);
+    try {
+      newDir.copy(csw, "d1", "d1", newIOContext(random));
+      fail("file does already exist");
+    } catch (IOException e) {
+      //
+    }
+    out.close();
+    assertEquals(1, csw.listAll().length);
+    assertEquals("d.xyz", csw.listAll()[0]);
+   
+    csw.close();
+
+    CompoundFileDirectory cfr = newDir.openCompoundInput("d.cfs", newIOContext(random));
+    assertEquals(1, cfr.listAll().length);
+    assertEquals("d.xyz", cfr.listAll()[0]);
+    cfr.close();
+    newDir.close();
+  }
+  
+  /** Creates a file of the specified size with sequential data. The first
+   *  byte is written as the start byte provided. All subsequent bytes are
+   *  computed as start + offset where offset is the number of the byte.
+   */
+  private void createSequenceFile(Directory dir, String name, byte start, int size) throws IOException {
+      IndexOutput os = dir.createOutput(name, newIOContext(random));
+      for (int i=0; i < size; i++) {
+          os.writeByte(start);
+          start ++;
+      }
+      os.close();
+  }
+
 }