Fire IndexChangedEvent on DirCache.commit()

Since we replaced GitIndex by DirCache JGit didn't fire
IndexChangedEvents anymore. For EGit this still worked with a high
latency since its RepositoryChangeScanner which is scheduled to
run each 10 seconds fires the event in case the index changes.
This scanner is meant to detect index changes induced by a different
process e.g. by calling "git add" from native git.

When the index is changed from within the same process we should fire
the event synchronously. Compare the index checksum on write to index
checksum when index was read earlier to determine if index really
changed. Use IndexChangedListener interface to keep DirCache decoupled
from Repository.

Change-Id: Id4311f7a7859ffe8738863b3d86c83c8b5f513af
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

org.eclipse.jgit.storage.dht/src/org/eclipse/jgit/storage/dht/DhtRepository.java

@@ -153,6 +153,11 @@ public class DhtRepository extends Repository {
 		refdb.clearCache();
 	}
 
+	@Override
+	public void notifyIndexChanged() {
+		// we do not support non-bare repositories yet
+	}
+
 	@Override
 	public String toString() {
 		return "DhtRepostitory[" + key + " / " + name + "]";

org.eclipse.jgit.test/tst/org/eclipse/jgit/dircache/DirCacheBuilderTest.java

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2008-2009, Google Inc.
+ * Copyright (C) 2011, Matthias Sohn <matthias.sohn@sap.com>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -49,9 +50,13 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.File;
 
+import org.eclipse.jgit.events.IndexChangedEvent;
+import org.eclipse.jgit.events.IndexChangedListener;
+import org.eclipse.jgit.events.ListenerList;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.RepositoryTestCase;
@@ -187,6 +192,73 @@ public class DirCacheBuilderTest extends RepositoryTestCase {
 		}
 	}
 
+	@Test
+	public void testBuildOneFile_Commit_IndexChangedEvent()
+			throws Exception {
+		final class ReceivedEventMarkerException extends RuntimeException {
+			private static final long serialVersionUID = 1L;
+			// empty
+		}
+
+		final String path = "a-file-path";
+		final FileMode mode = FileMode.REGULAR_FILE;
+		// "old" date in 2008
+		final long lastModified = 1218123387057L;
+		final int length = 1342;
+		DirCacheEntry entOrig;
+		boolean receivedEvent = false;
+
+		DirCache dc = db.lockDirCache();
+		IndexChangedListener listener = new IndexChangedListener() {
+
+			public void onIndexChanged(IndexChangedEvent event) {
+				throw new ReceivedEventMarkerException();
+			}
+		};
+
+		ListenerList l = db.getListenerList();
+		l.addIndexChangedListener(listener);
+		DirCacheBuilder b = dc.builder();
+
+		entOrig = new DirCacheEntry(path);
+		entOrig.setFileMode(mode);
+		entOrig.setLastModified(lastModified);
+		entOrig.setLength(length);
+		b.add(entOrig);
+		try {
+			b.commit();
+		} catch (ReceivedEventMarkerException e) {
+			receivedEvent = true;
+		}
+		if (!receivedEvent)
+			fail("did not receive IndexChangedEvent");
+
+		// do the same again, as this doesn't change index compared to first
+		// round we should get no event this time
+		dc = db.lockDirCache();
+		listener = new IndexChangedListener() {
+
+			public void onIndexChanged(IndexChangedEvent event) {
+				throw new ReceivedEventMarkerException();
+			}
+		};
+
+		l = db.getListenerList();
+		l.addIndexChangedListener(listener);
+		b = dc.builder();
+
+		entOrig = new DirCacheEntry(path);
+		entOrig.setFileMode(mode);
+		entOrig.setLastModified(lastModified);
+		entOrig.setLength(length);
+		b.add(entOrig);
+		try {
+			b.commit();
+		} catch (ReceivedEventMarkerException e) {
+			fail("unexpected IndexChangedEvent");
+		}
+	}
+
 	@Test
 	public void testFindSingleFile() throws Exception {
 		final String path = "a-file-path";

org.eclipse.jgit/src/org/eclipse/jgit/dircache/DirCache.java

@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2008-2010, Google Inc.
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ * Copyright (C) 2011, Matthias Sohn <matthias.sohn@sap.com>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -63,6 +64,8 @@ import java.util.Comparator;
 import org.eclipse.jgit.JGitText;
 import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.errors.UnmergedPathException;
+import org.eclipse.jgit.events.IndexChangedEvent;
+import org.eclipse.jgit.events.IndexChangedListener;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectInserter;
@@ -94,6 +97,8 @@ public class DirCache {
 
 	private static final DirCacheEntry[] NO_ENTRIES = {};
 
+	private static final byte[] NO_CHECKSUM = {};
+
 	static final Comparator<DirCacheEntry> ENT_CMP = new Comparator<DirCacheEntry>() {
 		public int compare(final DirCacheEntry o1, final DirCacheEntry o2) {
 			final int cr = cmp(o1, o2);
@@ -203,6 +208,39 @@ public class DirCache {
 		return c;
 	}
 
+	/**
+	 * Create a new in-core index representation, lock it, and read from disk.
+	 * <p>
+	 * The new index will be locked and then read before it is returned to the
+	 * caller. Read failures are reported as exceptions and therefore prevent
+	 * the method from returning a partially populated index. On read failure,
+	 * the lock is released.
+	 *
+	 * @param indexLocation
+	 *            location of the index file on disk.
+	 * @param fs
+	 *            the file system abstraction which will be necessary to perform
+	 *            certain file system operations.
+	 * @param indexChangedListener
+	 *            listener to be informed when DirCache is committed
+	 * @return a cache representing the contents of the specified index file (if
+	 *         it exists) or an empty cache if the file does not exist.
+	 * @throws IOException
+	 *             the index file is present but could not be read, or the lock
+	 *             could not be obtained.
+	 * @throws CorruptObjectException
+	 *             the index file is using a format or extension that this
+	 *             library does not support.
+	 */
+	public static DirCache lock(final File indexLocation, final FS fs,
+			IndexChangedListener indexChangedListener)
+			throws CorruptObjectException,
+			IOException {
+		DirCache c = lock(indexLocation, fs);
+		c.registerIndexChangedListener(indexChangedListener);
+		return c;
+	}
+
 	/** Location of the current version of the index file. */
 	private final File liveFile;
 
@@ -224,6 +262,15 @@ public class DirCache {
 	/** Keep track of whether the index has changed or not */
 	private FileSnapshot snapshot;
 
+	/** index checksum when index was read from disk */
+	private byte[] readIndexChecksum;
+
+	/** index checksum when index was written to disk */
+	private byte[] writeIndexChecksum;
+
+	/** listener to be informed on commit */
+	private IndexChangedListener indexChangedListener;
+
 	/**
 	 * Create a new in-core index representation.
 	 * <p>
@@ -330,6 +377,7 @@ public class DirCache {
 		sortedEntries = NO_ENTRIES;
 		entryCnt = 0;
 		tree = null;
+		readIndexChecksum = NO_CHECKSUM;
 	}
 
 	private void readFrom(final InputStream inStream) throws IOException,
@@ -412,8 +460,8 @@ public class DirCache {
 			}
 		}
 
-		final byte[] exp = md.digest();
+		readIndexChecksum = md.digest();
-		if (!Arrays.equals(exp, hdr)) {
+		if (!Arrays.equals(readIndexChecksum, hdr)) {
 			throw new CorruptObjectException(JGitText.get().DIRCChecksumMismatch);
 		}
 	}
@@ -544,8 +592,8 @@ public class DirCache {
 			dos.write(tmp, 0, 8);
 			bb.writeTo(dos, null);
 		}
-
+		writeIndexChecksum = foot.digest();
-		os.write(foot.digest());
+		os.write(writeIndexChecksum);
 		os.close();
 	}
 
@@ -567,6 +615,9 @@ public class DirCache {
 		if (!tmp.commit())
 			return false;
 		snapshot = tmp.getCommitSnapshot();
+		if (indexChangedListener != null
+				&& !Arrays.equals(readIndexChecksum, writeIndexChecksum))
+			indexChangedListener.onIndexChanged(new IndexChangedEvent());
 		return true;
 	}
 
@@ -794,4 +845,8 @@ public class DirCache {
 		}
 		return false;
 	}
+
+	private void registerIndexChangedListener(IndexChangedListener listener) {
+		this.indexChangedListener = listener;
+	}
 }

org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java

@@ -69,6 +69,8 @@ import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.errors.NoWorkTreeException;
 import org.eclipse.jgit.errors.RevisionSyntaxException;
+import org.eclipse.jgit.events.IndexChangedEvent;
+import org.eclipse.jgit.events.IndexChangedListener;
 import org.eclipse.jgit.events.ListenerList;
 import org.eclipse.jgit.events.RepositoryEvent;
 import org.eclipse.jgit.revwalk.RevBlob;
@@ -883,7 +885,15 @@ public abstract class Repository {
 	 */
 	public DirCache lockDirCache() throws NoWorkTreeException,
 			CorruptObjectException, IOException {
-		return DirCache.lock(getIndexFile(), getFS());
+		// we want DirCache to inform us so that we can inform registered
+		// listeners about index changes
+		IndexChangedListener l = new IndexChangedListener() {
+
+			public void onIndexChanged(IndexChangedEvent event) {
+				notifyIndexChanged();
+			}
+		};
+		return DirCache.lock(getIndexFile(), getFS(), l);
 	}
 
 	static byte[] gitInternalSlash(byte[] bytes) {
@@ -1065,6 +1075,11 @@ public abstract class Repository {
 	 */
 	public abstract void scanForRepoChanges() throws IOException;
 
+	/**
+	 * Notify that the index changed
+	 */
+	public abstract void notifyIndexChanged();
+
 	/**
 	 * @param refName
 	 *

org.eclipse.jgit/src/org/eclipse/jgit/storage/file/FileRepository.java

@@ -393,10 +393,14 @@ public class FileRepository extends Repository {
 		File indexFile = getIndexFile();
 		if (snapshot == null)
 			snapshot = FileSnapshot.save(indexFile);
-		else if (snapshot.isModified(indexFile)) {
+		else if (snapshot.isModified(indexFile))
-			snapshot = FileSnapshot.save(indexFile);
+			notifyIndexChanged();
-			fireEvent(new IndexChangedEvent());
 	}
+
+	@Override
+	public void notifyIndexChanged() {
+		snapshot = FileSnapshot.save(getIndexFile());
+		fireEvent(new IndexChangedEvent());
 	}
 
 	/**