Let RefDirectory use FileSnapShot to handle fast updates

Since this change may affect performance and memory consumption on every
access to a loose ref I explicitly made it a RFC to collect opinions.

Previously RefDirectory.scanRef() was not detecting an update of a
loose ref when the update didn't changed the modification time of
the backing file. RefDirectory cached loose refs and the way to detect
outdated cache entries was to compare lastmodification timestamp on the
file representing the ref. If two updates to the same ref happen faster
than the filesystem-timer granularity (for linux this is 2 seconds)
there is the possiblity that we don't detect the update.

Because of this bug EGit's PushOperationTest only works with 2 second
sleeps inside.

This change let RefDirectory use FileSnapshot to detect such situations.
FileSnapshot helps to remember when a file was last read from disk and
therefore enables to decide when to load a file from disk although
modification time has not changed.

Change-Id: I03b9a137af097ec69c4c5e2eaa512d2bdd7fe080
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Chris Aniszczyk <caniszczyk@gmail.com>

2 files changed, 43 insertions, 25 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
index 6a28605ed1..760d31a349 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/PushCommandTest.java
@@ -152,4 +152,47 @@ public class PushCommandTest extends RepositoryTestCase {
 		assertEquals(commit2.getId(), db2.resolve(branch));
 	}
 
+	/**
+	 * Check that pushes over file protocol lead to appropriate ref-updates.
+	 *
+	 * @throws Exception
+	 */
+	@Test
+	public void testPushRefUpdate() throws Exception {
+		Git git = new Git(db);
+		Git git2 = new Git(createBareRepository());
+
+		final StoredConfig config = git.getRepository().getConfig();
+		RemoteConfig remoteConfig = new RemoteConfig(config, "test");
+		URIish uri = new URIish(git2.getRepository().getDirectory().toURI()
+				.toURL());
+		remoteConfig.addURI(uri);
+		remoteConfig.addPushRefSpec(new RefSpec("+refs/heads/*:refs/heads/*"));
+		remoteConfig.update(config);
+		config.save();
+
+		writeTrashFile("f", "content of f");
+		git.add().addFilepattern("f").call();
+		RevCommit commit = git.commit().setMessage("adding f").call();
+
+		assertEquals(null, git2.getRepository().resolve("refs/heads/master"));
+		git.push().setRemote("test").call();
+		assertEquals(commit.getId(),
+				git2.getRepository().resolve("refs/heads/master"));
+
+		git.branchCreate().setName("refs/heads/test").call();
+		git.checkout().setName("refs/heads/test").call();
+
+
+		for (int i = 0; i < 6; i++) {
+			writeTrashFile("f" + i, "content of f" + i);
+			git.add().addFilepattern("f" + i).call();
+			commit = git.commit().setMessage("adding f" + i).call();
+			git.push().setRemote("test").call();
+			git2.getRepository().getAllRefs();
+			assertEquals("failed to update on attempt " + i, commit.getId(),
+					git2.getRepository().resolve("refs/heads/test"));
+
+		}
+	}
 }
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
index 56e5549b84..dc2ccb97fb 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefDirectoryTest.java
@@ -169,7 +169,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 		Ref head;
 
 		writeLooseRef(HEAD, A);
-		BUG_WorkAroundRacyGitIssues(HEAD);
 
 		all = refdir.getRefs(RefDatabase.ALL);
 		assertEquals(1, all.size());
@@ -190,7 +189,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 
 		writeLooseRef(HEAD, A);
 		writeLooseRef("refs/heads/master", B);
-		BUG_WorkAroundRacyGitIssues(HEAD);
 
 		all = refdir.getRefs(RefDatabase.ALL);
 		assertEquals(2, all.size());
@@ -328,7 +326,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 		assertTrue(heads.containsKey("refs/heads/C"));
 
 		writeLooseRef("refs/heads/B", "FAIL\n");
-		BUG_WorkAroundRacyGitIssues("refs/heads/B");
 
 		heads = refdir.getRefs(RefDatabase.ALL);
 		assertEquals(2, heads.size());
@@ -547,7 +544,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 		assertEquals(A, all.get(HEAD).getObjectId());
 
 		writeLooseRef("refs/heads/master", B);
-		BUG_WorkAroundRacyGitIssues("refs/heads/master");
 		all = refdir.getRefs(RefDatabase.ALL);
 		assertEquals(B, all.get(HEAD).getObjectId());
 	}
@@ -561,7 +557,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 		assertEquals(A, all.get(HEAD).getObjectId());
 
 		writeLooseRef("refs/heads/master", B);
-		BUG_WorkAroundRacyGitIssues("refs/heads/master");
 
 		Ref master = refdir.getRef("refs/heads/master");
 		assertEquals(B, master.getObjectId());
@@ -760,7 +755,6 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 
 		writeLooseRef("refs/5", "ref: refs/6\n");
 		writeLooseRef("refs/6", "ref: refs/end\n");
-		BUG_WorkAroundRacyGitIssues("refs/5");
 		all = refdir.getRefs(RefDatabase.ALL);
 		r = all.get("refs/1");
 		assertNull("mising 1 due to cycle", r);
@@ -1078,23 +1072,4 @@ public class RefDirectoryTest extends LocalDiskRepositoryTestCase {
 		File path = new File(diskRepo.getDirectory(), name);
 		assertTrue("deleted " + name, path.delete());
 	}
-
-	/**
-	 * Kick the timestamp of a local file.
-	 * <p>
-	 * We shouldn't have to make these method calls. The cache is using file
-	 * system timestamps, and on many systems unit tests run faster than the
-	 * modification clock. Dumping the cache after we make an edit behind
-	 * RefDirectory's back allows the tests to pass.
-	 *
-	 * @param name
-	 *            the file in the repository to force a time change on.
-	 */
-	private void BUG_WorkAroundRacyGitIssues(String name) {
-		File path = new File(diskRepo.getDirectory(), name);
-		long old = path.lastModified();
-		long set = 1250379778668L; // Sat Aug 15 20:12:58 GMT-03:30 2009
-		path.setLastModified(set);
-		assertTrue("time changed", old != path.lastModified());
-	}
 }