Include size when comparing FileSnapshot

Due to finite filesystem timestamp resolution the last modified
timestamp of files cannot detect file changes which happened in the
immediate past (less than one filesystem timer tick ago).

Read and consider file size also, so that differing file size can help
to more accurately detect file changes without reading the file content.
Use bulk read to avoid multiple stat calls to retrieve file attributes.

Change-Id: I974288fff78ac78c52245d9218b5639603f67a46
Signed-off-by: Luca Milanesio <luca.milanesio@gmail.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/FileSnapshotTest.java

@@ -42,6 +42,7 @@
  */
 package org.eclipse.jgit.internal.storage.file;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -98,6 +99,27 @@ public class FileSnapshotTest {
 		assertTrue(save.isModified(f1));
 	}
 
+	/**
+	 * Check that file is modified by looking at its size.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void tesIsModifiedBySameLastModifiedAndDifferentSize() throws Exception {
+		File f1 = createFile("foo", "lorem".getBytes());
+		File f2 = createFile("bar", "lorem ipsum".getBytes());
+
+		f1.setLastModified(f2.lastModified()); // Make sure f1 and f2 have the same lastModified
+		FileSnapshot save = FileSnapshot.save(f1);
+
+		// Make sure that the modified and read timestamps are far enough, so that
+		// check is done by size
+		Thread.sleep(3000L);
+
+		assertEquals(save.lastModified(), f2.lastModified());
+		assertTrue(save.isModified(f2));
+	}
+
 	/**
 	 * Create a file, wait long enough and verify that it has not been modified.
 	 * 3.5 seconds mean any difference between file system timestamp and system
@@ -152,10 +174,22 @@ public class FileSnapshotTest {
 		return f;
 	}
 
+	private File createFile(String string, byte[] content) throws IOException {
+		File f = createFile(string);
+		append(f, content);
+		return f;
+	}
+
 	private static void append(File f, byte b) throws IOException {
+		append(f, new byte[] { b });
+	}
+
+	private static void append(File f, byte[] content) throws IOException {
 		FileOutputStream os = new FileOutputStream(f, true);
 		try {
-			os.write(b);
+			for (byte b : content) {
+				os.write(b);
+			}
 		} finally {
 			os.close();
 		}

org.eclipse.jgit/.settings/.api_filters

@@ -22,4 +22,12 @@
             </message_arguments>
         </filter>
     </resource>
+    <resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS">
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="4.5.6"/>
+                <message_argument value="fileAttributes(File)"/>
+            </message_arguments>
+        </filter>
+    </resource>
 </component>

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/FileSnapshot.java

@@ -45,6 +45,7 @@ package org.eclipse.jgit.internal.storage.file;
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.text.DateFormat;
 import java.text.SimpleDateFormat;
 import java.util.Date;
@@ -69,6 +70,13 @@ import org.eclipse.jgit.util.FS;
  * file is less than 3 seconds ago.
  */
 public class FileSnapshot {
+	/**
+	 * An unknown file size.
+	 *
+	 * This value is used when a comparison needs to happen purely on the lastUpdate.
+	 */
+	public static final long UNKNOWN_SIZE = -1;
+
 	/**
 	 * A FileSnapshot that is considered to always be modified.
 	 * <p>
@@ -76,7 +84,7 @@ public class FileSnapshot {
 	 * file, but only after {@link #isModified(File)} gets invoked. The returned
 	 * snapshot contains only invalid status information.
 	 */
-	public static final FileSnapshot DIRTY = new FileSnapshot(-1, -1);
+	public static final FileSnapshot DIRTY = new FileSnapshot(-1, -1, UNKNOWN_SIZE);
 
 	/**
 	 * A FileSnapshot that is clean if the file does not exist.
@@ -85,7 +93,7 @@ public class FileSnapshot {
 	 * file to be clean. {@link #isModified(File)} will return false if the file
 	 * path does not exist.
 	 */
-	public static final FileSnapshot MISSING_FILE = new FileSnapshot(0, 0) {
+	public static final FileSnapshot MISSING_FILE = new FileSnapshot(0, 0, 0) {
 		@Override
 		public boolean isModified(File path) {
 			return FS.DETECTED.exists(path);
@@ -105,12 +113,16 @@ public class FileSnapshot {
 	public static FileSnapshot save(File path) {
 		long read = System.currentTimeMillis();
 		long modified;
+		long size;
 		try {
-			modified = FS.DETECTED.lastModified(path);
+			BasicFileAttributes fileAttributes = FS.DETECTED.fileAttributes(path);
+			modified = fileAttributes.lastModifiedTime().toMillis();
+			size = fileAttributes.size();
 		} catch (IOException e) {
 			modified = path.lastModified();
+			size = path.length();
 		}
-		return new FileSnapshot(read, modified);
+		return new FileSnapshot(read, modified, size);
 	}
 
 	/**
@@ -126,7 +138,7 @@ public class FileSnapshot {
 	 */
 	public static FileSnapshot save(long modified) {
 		final long read = System.currentTimeMillis();
-		return new FileSnapshot(read, modified);
+		return new FileSnapshot(read, modified, -1);
 	}
 
 	/** Last observed modification time of the path. */
@@ -138,10 +150,16 @@ public class FileSnapshot {
 	/** True once {@link #lastRead} is far later than {@link #lastModified}. */
 	private boolean cannotBeRacilyClean;
 
-	private FileSnapshot(long read, long modified) {
+	/** Underlying file-system size in bytes.
+	 *
+	 * When set to {@link #UNKNOWN_SIZE} the size is not considered for modification checks. */
+	private final long size;
+
+	private FileSnapshot(long read, long modified, long size) {
 		this.lastRead = read;
 		this.lastModified = modified;
 		this.cannotBeRacilyClean = notRacyClean(read);
+		this.size = size;
 	}
 
 	/**
@@ -151,6 +169,13 @@ public class FileSnapshot {
 		return lastModified;
 	}
 
+	/**
+	 * @return file size in bytes of last snapshot update
+	 */
+	public long size() {
+		return size;
+	}
+
 	/**
 	 * Check if the path may have been modified since the snapshot was saved.
 	 *
@@ -160,12 +185,16 @@ public class FileSnapshot {
 	 */
 	public boolean isModified(File path) {
 		long currLastModified;
+		long currSize;
 		try {
-			currLastModified = FS.DETECTED.lastModified(path);
+			BasicFileAttributes fileAttributes = FS.DETECTED.fileAttributes(path);
+			currLastModified = fileAttributes.lastModifiedTime().toMillis();
+			currSize = fileAttributes.size();
 		} catch (IOException e) {
 			currLastModified = path.lastModified();
+			currSize = path.length();
 		}
-		return isModified(currLastModified);
+		return (currSize != UNKNOWN_SIZE && currSize != size) || isModified(currLastModified);
 	}
 
 	/**

org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java

@@ -52,6 +52,7 @@ import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.nio.charset.Charset;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.text.MessageFormat;
@@ -417,6 +418,19 @@ public abstract class FS {
 	 */
 	public abstract boolean retryFailedLockFileCommit();
 
+	/**
+	 * Return all the attributes of a file, without following symbolic links.
+	 *
+	 * @param file
+	 * @return {@link BasicFileAttributes} of the file
+	 * @throws IOException in case of any I/O errors accessing the file
+	 *
+	 * @since 4.5.6
+	 */
+	public BasicFileAttributes fileAttributes(File file) throws IOException {
+		return FileUtils.fileAttributes(file);
+	}
+
 	/**
 	 * Determine the user's home directory (location where preferences are).
 	 *

org.eclipse.jgit/src/org/eclipse/jgit/util/FileUtils.java

@@ -564,6 +564,19 @@ public class FileUtils {
 				.toMillis();
 	}
 
+	/**
+	 * Return all the attributes of a file, without following symbolic links.
+	 *
+	 * @param file
+	 * @return {@link BasicFileAttributes} of the file
+	 * @throws IOException in case of any I/O errors accessing the file
+	 *
+	 * @since 4.5.6
+	 */
+	static BasicFileAttributes fileAttributes(File file) throws IOException {
+		return Files.readAttributes(file.toPath(), BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS);
+	}
+
 	/**
 	 * @param file
 	 * @param time