Merge "Let RefDirectory use FileSnapShot to handle fast updates"

4 files changed, 98 insertions, 49 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());
-	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java
index bbec80c86c..ce556a7920 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileSnapshot.java
@@ -101,6 +101,22 @@ public class FileSnapshot {
 		return new FileSnapshot(read, modified);
 	}
 
+	/**
+	 * Record a snapshot for a file for which the last modification time is
+	 * already known.
+	 * <p>
+	 * This method should be invoked before the file is accessed.
+	 *
+	 * @param modified
+	 *            the last modification time of the file
+	 *
+	 * @return the snapshot.
+	 */
+	public static FileSnapshot save(long modified) {
+		final long read = System.currentTimeMillis();
+		return new FileSnapshot(read, modified);
+	}
+
 	/** Last observed modification time of the path. */
 	private final long lastModified;
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
index cd199dcf92..90fd38bdeb 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/storage/file/RefDirectory.java
@@ -842,19 +842,22 @@ public class RefDirectory extends RefDatabase {
 		return n;
 	}
 
+	@SuppressWarnings("null")
 	private LooseRef scanRef(LooseRef ref, String name) throws IOException {
 		final File path = fileFor(name);
-		final long modified = path.lastModified();
+		FileSnapshot currentSnapshot = null;
 
 		if (ref != null) {
-			if (ref.getLastModified() == modified)
+			currentSnapshot = ref.getSnapShot();
+			if (!currentSnapshot.isModified(path))
 				return ref;
 			name = ref.getName();
-		} else if (modified == 0)
+		} else if (!path.exists())
 			return null;
 
 		final int limit = 4096;
 		final byte[] buf;
+		FileSnapshot otherSnapshot = FileSnapshot.save(path);
 		try {
 			buf = IO.readSome(path, limit);
 		} catch (FileNotFoundException noFile) {
@@ -877,7 +880,12 @@ public class RefDirectory extends RefDatabase {
 				throw new IOException(MessageFormat.format(JGitText.get().notARef, name, content));
 			}
 			final String target = RawParseUtils.decode(buf, 5, n);
-			return newSymbolicRef(modified, name, target);
+			if (ref != null && ref.isSymbolic()
+					&& ref.getTarget().getName().equals(target)) {
+				currentSnapshot.setClean(otherSnapshot);
+				return ref;
+			}
+			return newSymbolicRef(path.lastModified(), name, target);
 		}
 
 		if (n < OBJECT_ID_STRING_LENGTH)
@@ -886,13 +894,19 @@ public class RefDirectory extends RefDatabase {
 		final ObjectId id;
 		try {
 			id = ObjectId.fromString(buf, 0);
+			if (ref != null && !ref.isSymbolic()
+					&& ref.getTarget().getObjectId().equals(id)) {
+				currentSnapshot.setClean(otherSnapshot);
+				return ref;
+			}
+
 		} catch (IllegalArgumentException notRef) {
 			while (0 < n && Character.isWhitespace(buf[n - 1]))
 				n--;
 			String content = RawParseUtils.decode(buf, 0, n);
 			throw new IOException(MessageFormat.format(JGitText.get().notARef, name, content));
 		}
-		return new LooseUnpeeled(modified, name, id);
+		return new LooseUnpeeled(path.lastModified(), name, id);
 	}
 
 	private static boolean isSymRef(final byte[] buf, int n) {
@@ -997,22 +1011,22 @@ public class RefDirectory extends RefDatabase {
 	}
 
 	private static interface LooseRef extends Ref {
-		long getLastModified();
+		FileSnapshot getSnapShot();
 
 		LooseRef peel(ObjectIdRef newLeaf);
 	}
 
 	private final static class LoosePeeledTag extends ObjectIdRef.PeeledTag
 			implements LooseRef {
-		private final long lastModified;
+		private final FileSnapshot snapShot;
 
 		LoosePeeledTag(long mtime, String refName, ObjectId id, ObjectId p) {
 			super(LOOSE, refName, id, p);
-			this.lastModified = mtime;
+			snapShot = FileSnapshot.save(mtime);
 		}
 
-		public long getLastModified() {
-			return lastModified;
+		public FileSnapshot getSnapShot() {
+			return snapShot;
 		}
 
 		public LooseRef peel(ObjectIdRef newLeaf) {
@@ -1022,15 +1036,15 @@ public class RefDirectory extends RefDatabase {
 
 	private final static class LooseNonTag extends ObjectIdRef.PeeledNonTag
 			implements LooseRef {
-		private final long lastModified;
+		private final FileSnapshot snapShot;
 
 		LooseNonTag(long mtime, String refName, ObjectId id) {
 			super(LOOSE, refName, id);
-			this.lastModified = mtime;
+			snapShot = FileSnapshot.save(mtime);
 		}
 
-		public long getLastModified() {
-			return lastModified;
+		public FileSnapshot getSnapShot() {
+			return snapShot;
 		}
 
 		public LooseRef peel(ObjectIdRef newLeaf) {
@@ -1040,37 +1054,38 @@ public class RefDirectory extends RefDatabase {
 
 	private final static class LooseUnpeeled extends ObjectIdRef.Unpeeled
 			implements LooseRef {
-		private final long lastModified;
+		private final FileSnapshot snapShot;
 
 		LooseUnpeeled(long mtime, String refName, ObjectId id) {
 			super(LOOSE, refName, id);
-			this.lastModified = mtime;
+			snapShot = FileSnapshot.save(mtime);
 		}
 
-		public long getLastModified() {
-			return lastModified;
+		public FileSnapshot getSnapShot() {
+			return snapShot;
 		}
 
 		public LooseRef peel(ObjectIdRef newLeaf) {
 			if (newLeaf.getPeeledObjectId() != null)
-				return new LoosePeeledTag(lastModified, getName(),
+				return new LoosePeeledTag(snapShot.lastModified(), getName(),
 						getObjectId(), newLeaf.getPeeledObjectId());
 			else
-				return new LooseNonTag(lastModified, getName(), getObjectId());
+				return new LooseNonTag(snapShot.lastModified(), getName(),
+						getObjectId());
 		}
 	}
 
 	private final static class LooseSymbolicRef extends SymbolicRef implements
 			LooseRef {
-		private final long lastModified;
+		private final FileSnapshot snapShot;
 
 		LooseSymbolicRef(long mtime, String refName, Ref target) {
 			super(refName, target);
-			this.lastModified = mtime;
+			snapShot = FileSnapshot.save(mtime);
 		}
 
-		public long getLastModified() {
-			return lastModified;
+		public FileSnapshot getSnapShot() {
+			return snapShot;
 		}
 
 		public LooseRef peel(ObjectIdRef newLeaf) {