LUCENE-3382: fix compound-file/NoSuchDirectoryException bugs in NRTCachingDir

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

lucene/contrib/CHANGES.txt

@@ -104,6 +104,11 @@ Bug Fixes
    
  * LUCENE-3347: XML query parser did not always incorporate boosts from
    UserQuery elements.  (Moogie, Uwe Schindler)
+   
+ * LUCENE-3382: Fixed a bug where NRTCachingDirectory's listAll() would wrongly
+   throw NoSuchDirectoryException when all files written so far have been
+   cached to RAM and the directory still has not yet been created on the
+   filesystem.  (Robert Muir)
 
 ======================= Lucene 3.3.0 =======================
 

lucene/contrib/misc/src/java/org/apache/lucene/store/NRTCachingDirectory.java

@@ -120,11 +120,23 @@ public class NRTCachingDirectory extends Directory {
     for(String f : cache.listAll()) {
       files.add(f);
     }
-    for(String f : delegate.listAll()) {
+    // LUCENE-1468: our NRTCachingDirectory will actually exist (RAMDir!),
-      // Cannot do this -- if lucene calls createOutput but
+    // but if the underlying delegate is an FSDir and mkdirs() has not
-      // file already exists then this falsely trips:
+    // yet been called, because so far everything is a cached write,
-      //assert !files.contains(f): "file \"" + f + "\" is in both dirs";
+    // in this case, we don't want to throw a NoSuchDirectoryException
-      files.add(f);
+    try {
+      for(String f : delegate.listAll()) {
+        // Cannot do this -- if lucene calls createOutput but
+        // file already exists then this falsely trips:
+        //assert !files.contains(f): "file \"" + f + "\" is in both dirs";
+        files.add(f);
+      }
+    } catch (NoSuchDirectoryException ex) {
+      // however, if there are no cached files, then the directory truly
+      // does not "exist"
+      if (files.isEmpty()) {
+        throw ex;
+      }
     }
     return files.toArray(new String[files.size()]);
   }
@@ -216,26 +228,22 @@ public class NRTCachingDirectory extends Directory {
     }
   }
 
+  // final due to LUCENE-3382: 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 synchronized CompoundFileDirectory openCompoundInput(String name, IOContext context) throws IOException {
+  public final CompoundFileDirectory openCompoundInput(String name, IOContext context) throws IOException {
-    if (cache.fileExists(name)) {
+    return super.openCompoundInput(name, context);
-      return cache.openCompoundInput(name, context);
-    } else {
-      return delegate.openCompoundInput(name, context);
-    }
   }
   
+  // final due to LUCENE-3382: 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 synchronized CompoundFileDirectory createCompoundOutput(String name, IOContext context)
+  public final CompoundFileDirectory createCompoundOutput(String name, IOContext context) throws IOException {
-      throws IOException {
+    return super.createCompoundOutput(name, context);
-    if (cache.fileExists(name)) {
-      throw new IOException("File " + name + "already exists");
-    } else {
-      return delegate.createCompoundOutput(name, context);
-    }
   }
 
-
   /** Close this directory, which flushes any cached files
    *  to the delegate and then closes the delegate. */
   @Override

lucene/contrib/misc/src/test/org/apache/lucene/store/TestNRTCachingDirectory.java

@@ -18,7 +18,9 @@ package org.apache.lucene.store;
  */
 
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.lucene.analysis.Analyzer;
@@ -117,4 +119,68 @@ public class TestNRTCachingDirectory extends LuceneTestCase {
     assertEquals(0, dir.listAll().length);
     dir.close();
   }
+  
+  // LUCENE-3382 -- make sure we get exception if the directory really does not exist.
+  public void testNoDir() throws Throwable {
+    Directory dir = new NRTCachingDirectory(newFSDirectory(_TestUtil.getTempDir("doesnotexist")), 2.0, 25.0);
+    try {
+      IndexReader.open(dir, true);
+      fail("did not hit expected exception");
+    } catch (NoSuchDirectoryException nsde) {
+      // expected
+    }
+    dir.close();
+  }
+  
+  // LUCENE-3382 test that we can add a file, and then when we call list() we get it back
+  public void testDirectoryFilter() throws IOException {
+    Directory dir = new NRTCachingDirectory(newFSDirectory(_TestUtil.getTempDir("foo")), 2.0, 25.0);
+    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-3382 test that delegate compound files correctly.
+  public void testCompoundFileAppendTwice() throws IOException {
+    Directory newDir = new NRTCachingDirectory(newDirectory(), 2.0, 25.0);
+    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();
+  }
 }