AbstractIOServiceProvider.getLastModified() returns correct values wh…

…en called after close()

* Get last-modified time from underlying File, not the RandomAccessFile.
* Clarified FileCacheable.getLastModified(): zero is returned when time can't be determined for any reason.
* Reverted code in FileCache that considered files with "lastModified == 0" as unchanged.

cdm/src/main/java/ucar/nc2/iosp/AbstractIOServiceProvider.java

@@ -33,14 +33,18 @@
 
 package ucar.nc2.iosp;
 
-import ucar.ma2.*;
+import ucar.ma2.Array;
+import ucar.ma2.InvalidRangeException;
+import ucar.ma2.Section;
+import ucar.ma2.StructureDataIterator;
+import ucar.nc2.NetcdfFile;
 import ucar.nc2.ParsedSectionSpec;
 import ucar.nc2.Structure;
-import ucar.nc2.NetcdfFile;
 import ucar.nc2.util.CancelTask;
 import ucar.unidata.io.RandomAccessFile;
 import ucar.unidata.util.Format;
 
+import java.io.File;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.channels.WritableByteChannel;
@@ -152,14 +156,19 @@ public boolean syncExtend() throws IOException {
   }
 
   /**
-   * Get last modified date of underlying file. If changes, will be discarded.
-   * @return a sequence number (typically file date), 0 if cannot change
+   * Returns the time that the underlying file(s) were last modified. If they've changed since they were stored in the
+   * cache, they will be closed and reopened with {@link ucar.nc2.util.cache.FileFactory}.
+   *
+   * @return  a {@code long} value representing the time the file(s) were last modified or {@code 0L} if the
+   *          last-modified time couldn't be determined for any reason.
    */
   public long getLastModified() {
-    if (raf != null) {
-      return raf.getLastModified();
+    if (location != null) {
+      File file = new File(location);
+      return file.lastModified();
+    } else {
+      return 0;
     }
-    return 0;
   }
 
   @Override

cdm/src/main/java/ucar/nc2/util/cache/FileCache.java

@@ -346,8 +346,8 @@ private FileCacheable acquireCacheOnly(Object hashKey) {
 
     // check if modified, remove if so
     if (want.ncfile != null) {
-      long lastModified = want.ncfile.getLastModified();  // Will be 0 if ncfile is closed.
-      boolean changed = lastModified != 0 && lastModified != want.lastModified;
+      long lastModified = want.ncfile.getLastModified();
+      boolean changed = lastModified != want.lastModified;
 
       if (cacheLog.isDebugEnabled() && changed)
         cacheLog.debug("FileCache " + name + ": acquire from cache " + hashKey + " " + want.ncfile.getLocation() + " was changed; discard");

cdm/src/main/java/ucar/nc2/util/cache/FileCacheARC.java

@@ -249,8 +249,8 @@ private FileCacheable acquireCacheOnly(Object hashKey) {
 
     // check if modified, remove if so
     if (want.ncfile != null) {
-      long lastModified = want.ncfile.getLastModified();  // Will be 0 if ncfile is closed.
-      boolean changed = lastModified != 0 && lastModified != wantCacheElem.lastModified.get();
+      long lastModified = want.ncfile.getLastModified();
+      boolean changed = lastModified != wantCacheElem.lastModified.get();
 
       if (changed) { // underlying file was modified
         if (cacheLog.isDebugEnabled())

cdm/src/main/java/ucar/nc2/util/cache/FileCacheable.java

@@ -46,26 +46,27 @@
  * @since Jun 2, 2008
  */
 public interface FileCacheable {
-
   /**
    * The location of the FileCacheable. This must be sufficient for FileFactory.factory() to create the FileCacheable object
    * @return location
    */
-  public String getLocation();
+  String getLocation();
 
   /**
    * Close the FileCacheable, release all resources.
    * Must call cache.release(this) if cache is not null.
    * @throws IOException on io error
    */
-  public void close() throws IOException;
+  void close() throws IOException;
 
   /**
-   * Get last modified date of underlying file(s).
-   * If changed since it was stored in the cache, it will be closed and recreated with FileFactory
-   * @return a sequence number (typically file date), 0 if cannot change
+   * Returns the time that the underlying file(s) were last modified. If they've changed since they were stored in the
+   * cache, they will be closed and reopened with {@link ucar.nc2.util.cache.FileFactory}.
+   *
+   * @return  a {@code long} value representing the time the file(s) were last modified or {@code 0L} if the
+   *          last-modified time couldn't be determined for any reason.
    */
-  public long getLastModified();
+  long getLastModified();
 
   /**
    * If the FileCache is not null, FileCacheable.close() must call FileCache.release()
@@ -80,13 +81,11 @@ public synchronized void close() throws java.io.IOException {
    *
    * @param fileCache must store this, use it on close as above.
    */
-  public void setFileCache( FileCacheIF fileCache);
+  void setFileCache( FileCacheIF fileCache);
 
   // release any resources like file handles
-  public void release() throws IOException;
+  void release() throws IOException;
 
   // reacquire any resources like file handles
-  public void reacquire() throws IOException;
-
-
-}
+  void reacquire() throws IOException;
+}

cdm/src/test/groovy/ucar/nc2/util/cache/ReacquireClosedDatasetSpec.groovy

@@ -1,7 +1,9 @@
 package ucar.nc2.util.cache
+
 import spock.lang.Specification
 import ucar.nc2.dataset.NetcdfDataset
 import ucar.unidata.test.util.TestDir
+
 /**
  * Tests caching behavior when datasets are closed and then reacquired.
  *
@@ -21,7 +23,7 @@ class ReacquireClosedDatasetSpec extends Specification {
 
     def "reacquire"() {
         when: 'Acquire and close dataset 4 times'
-        (1..4).each { println ''
+        (1..4).each {
             NetcdfDataset.acquireDataset(TestDir.cdmLocalTestDataDir + "jan.nc", null).close()
         }
 
@@ -33,6 +35,7 @@ class ReacquireClosedDatasetSpec extends Specification {
         // This is kludgy, but FileCache doesn't provide getHits() or getMisses() methods.
         formatter.toString().trim() ==~ /hits= 3 miss= 1 nfiles= \d+ elems= \d+/
 
-        // Prior to a bug fix in FileCache, this would record 0 hits and 4 misses.
+        // Prior to 2016-03-09 bug fix in AbstractIOServiceProvider.getLastModified(),
+        // this would record 0 hits and 4 misses.
     }
 }