Ensure ReflogWriter only works with a RefDirectory

The ReflogWriter constructor just took a Repository and called
getDirectory() on it to figure out the reflog dirs, but not all
Repository instances use this storage format for reflogs, so it's
incorrect to attempt to use ReflogWriter when there is not a
RefDirectory directly involved. In practice, ReflogWriter was mostly
only used by the implementation of RefDirectory, so enforcing this is
mostly just shuffling around calls in the same internal package.

The one exception is StashDropCommand, which writes to a reflog lock
file directly. This was a reasonable implementation decision, because
there is no general reflog interface in JGit beyond using
(Batch)RefUpdate to write new entries to the reflog. So to implement
"git stash drop <N>", which removes an arbitrary element from the
reflog, it's fair to fall back to the RefDirectory implementation.
Creating and using a more general interface is well beyond the scope of
this change.

That said, the old behavior of writing out the reflog file even if
that's not the reflog format used by the given Repository is clearly
wrong. Fail fast in this case instead.

Change-Id: I9bd4b047bc3e28a5607fd346ec2400dde9151730

8 files changed, 66 insertions, 63 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ReflogWriterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ReflogWriterTest.java
index 89b969e3f1..7f40bae556 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ReflogWriterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/file/ReflogWriterTest.java
@@ -61,7 +61,8 @@ public class ReflogWriterTest extends SampleDataRepositoryTestCase {
 
 	@Test
 	public void shouldFilterLineFeedFromMessage() throws Exception {
-		ReflogWriter writer = new ReflogWriter(db);
+		ReflogWriter writer =
+				new ReflogWriter((RefDirectory) db.getRefDatabase());
 		PersonIdent ident = new PersonIdent("John Doe", "john@doe.com",
 				1243028200000L, 120);
 		ObjectId oldId = ObjectId
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 92bd1473e8..43dd9482de 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -626,6 +626,7 @@ stashCommitIncorrectNumberOfParents=Stashed commit ''{0}'' does have {1} parent
 stashDropDeleteRefFailed=Deleting stash reference failed with result: {0}
 stashDropFailed=Dropping stashed commit failed
 stashDropMissingReflog=Stash reflog does not contain entry ''{0}''
+stashDropNotSupported=Dropping stash not supported on this ref backend
 stashFailed=Stashing local changes did not successfully complete
 stashResolveFailed=Reference ''{0}'' does not resolve to stashed commit
 statelessRPCRequiresOptionToBeEnabled=stateless RPC requires {0} to be enabled
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashDropCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashDropCommand.java
index e215bdf7d3..85e7b3d298 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/StashDropCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/StashDropCommand.java
@@ -56,6 +56,7 @@ import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.api.errors.RefNotFoundException;
 import org.eclipse.jgit.errors.LockFailedException;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.storage.file.RefDirectory;
 import org.eclipse.jgit.internal.storage.file.ReflogWriter;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
@@ -68,6 +69,9 @@ import org.eclipse.jgit.util.FileUtils;
 
 /**
  * Command class to delete a stashed commit reference
+ * <p>
+ * Currently only supported on a traditional file repository using
+ * one-file-per-ref reflogs.
  *
  * @see <a href="http://www.kernel.org/pub/software/scm/git/docs/git-stash.html"
  *      >Git documentation about Stash</a>
@@ -84,6 +88,10 @@ public class StashDropCommand extends GitCommand<ObjectId> {
 	 */
 	public StashDropCommand(Repository repo) {
 		super(repo);
+		if (!(repo.getRefDatabase() instanceof RefDirectory)) {
+			throw new UnsupportedOperationException(
+					JGitText.get().stashDropNotSupported);
+		}
 	}
 
 	/**
@@ -205,10 +213,11 @@ public class StashDropCommand extends GitCommand<ObjectId> {
 			return null;
 		}
 
-		ReflogWriter writer = new ReflogWriter(repo, true);
+		RefDirectory refdb = (RefDirectory) repo.getRefDatabase();
+		ReflogWriter writer = new ReflogWriter(refdb, true);
 		String stashLockRef = ReflogWriter.refLockFor(R_STASH);
-		File stashLockFile = writer.logFor(stashLockRef);
-		File stashFile = writer.logFor(R_STASH);
+		File stashLockFile = refdb.logFor(stashLockRef);
+		File stashFile = refdb.logFor(R_STASH);
 		if (stashLockFile.exists())
 			throw new JGitInternalException(JGitText.get().stashDropFailed,
 					new LockFailedException(stashFile));
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 36e09680d6..6b3631601e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -685,6 +685,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String stashDropDeleteRefFailed;
 	/***/ public String stashDropFailed;
 	/***/ public String stashDropMissingReflog;
+	/***/ public String stashDropNotSupported;
 	/***/ public String stashFailed;
 	/***/ public String stashResolveFailed;
 	/***/ public String statelessRPCRequiresOptionToBeEnabled;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java
index 4309089b72..0bb502dfbf 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java
@@ -416,7 +416,7 @@ class PackedBatchRefUpdate extends BatchRefUpdate {
 
 			if (cmd.getType() == ReceiveCommand.Type.DELETE) {
 				try {
-					RefDirectory.delete(w.logFor(name), RefDirectory.levelsIn(name));
+					RefDirectory.delete(refdb.logFor(name), RefDirectory.levelsIn(name));
 				} catch (IOException e) {
 					// Ignore failures, see below.
 				}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
index 59f4e50f91..105efe7289 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectory.java
@@ -48,6 +48,7 @@ package org.eclipse.jgit.internal.storage.file;
 
 import static org.eclipse.jgit.lib.Constants.CHARSET;
 import static org.eclipse.jgit.lib.Constants.HEAD;
+import static org.eclipse.jgit.lib.Constants.LOGS;
 import static org.eclipse.jgit.lib.Constants.OBJECT_ID_STRING_LENGTH;
 import static org.eclipse.jgit.lib.Constants.PACKED_REFS;
 import static org.eclipse.jgit.lib.Constants.R_HEADS;
@@ -154,6 +155,10 @@ public class RefDirectory extends RefDatabase {
 
 	final File packedRefsFile;
 
+	final File logsDir;
+
+	final File logsRefsDir;
+
 	/**
 	 * Immutable sorted list of loose references.
 	 * <p>
@@ -205,8 +210,10 @@ public class RefDirectory extends RefDatabase {
 		final FS fs = db.getFS();
 		parent = db;
 		gitDir = db.getDirectory();
-		logWriter = new ReflogWriter(db);
+		logWriter = new ReflogWriter(this);
 		refsDir = fs.resolve(gitDir, R_REFS);
+		logsDir = fs.resolve(gitDir, LOGS);
+		logsRefsDir = fs.resolve(gitDir, LOGS + '/' + R_REFS);
 		packedRefsFile = fs.resolve(gitDir, PACKED_REFS);
 
 		looseRefs.set(RefList.<LooseRef> emptyList());
@@ -221,6 +228,22 @@ public class RefDirectory extends RefDatabase {
 		return logWriter;
 	}
 
+	/**
+	 * Locate the log file on disk for a single reference name.
+	 *
+	 * @param name
+	 *            name of the ref, relative to the Git repository top level
+	 *            directory (so typically starts with refs/).
+	 * @return the log file location.
+	 */
+	public File logFor(String name) {
+		if (name.startsWith(R_REFS)) {
+			name = name.substring(R_REFS.length());
+			return new File(logsRefsDir, name);
+		}
+		return new File(logsDir, name);
+	}
+
 	@Override
 	public void create() throws IOException {
 		FileUtils.mkdir(refsDir);
@@ -654,7 +677,7 @@ public class RefDirectory extends RefDatabase {
 		} while (!looseRefs.compareAndSet(curLoose, newLoose));
 
 		int levels = levelsIn(name) - 2;
-		delete(logWriter.logFor(name), levels);
+		delete(logFor(name), levels);
 		if (dst.getStorage().isLoose()) {
 			update.unlock();
 			delete(fileFor(name), levels);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectoryRename.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectoryRename.java
index 4b803a5144..09456c85c6 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectoryRename.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/RefDirectoryRename.java
@@ -188,8 +188,8 @@ class RefDirectoryRename extends RefRename {
 	}
 
 	private boolean renameLog(RefUpdate src, RefUpdate dst) {
-		File srcLog = refdb.getLogWriter().logFor(src.getName());
-		File dstLog = refdb.getLogWriter().logFor(dst.getName());
+		File srcLog = refdb.logFor(src.getName());
+		File dstLog = refdb.logFor(dst.getName());
 
 		if (!srcLog.exists())
 			return true;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java
index 0213c1021a..15fbe98209 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/ReflogWriter.java
@@ -46,7 +46,6 @@
 package org.eclipse.jgit.internal.storage.file;
 
 import static org.eclipse.jgit.lib.Constants.HEAD;
-import static org.eclipse.jgit.lib.Constants.LOGS;
 import static org.eclipse.jgit.lib.Constants.R_HEADS;
 import static org.eclipse.jgit.lib.Constants.R_REFS;
 import static org.eclipse.jgit.lib.Constants.R_REMOTES;
@@ -68,11 +67,12 @@ import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.RefUpdate;
 import org.eclipse.jgit.lib.ReflogEntry;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.util.FS;
 import org.eclipse.jgit.util.FileUtils;
 
-/** Utility for writing reflog entries. */
+/**
+ * Utility for writing reflog entries using the traditional one-file-per-log
+ * format.
+ */
 public class ReflogWriter {
 
 	/**
@@ -87,80 +87,47 @@ public class ReflogWriter {
 		return name + LockFile.SUFFIX;
 	}
 
-	private final Repository parent;
-
-	private final File logsDir;
-
-	private final File logsRefsDir;
+	private final RefDirectory refdb;
 
 	private final boolean forceWrite;
 
 	/**
-	 * Create writer for repository.
+	 * Create writer for ref directory.
 	 *
-	 * @param repository
+	 * @param refdb
 	 */
-	public ReflogWriter(Repository repository) {
-		this(repository, false);
+	public ReflogWriter(RefDirectory refdb) {
+		this(refdb, false);
 	}
 
 	/**
-	 * Create writer for repository.
+	 * Create writer for ref directory.
 	 *
-	 * @param repository
+	 * @param refdb
 	 * @param forceWrite
 	 *            true to write to disk all entries logged, false to respect the
 	 *            repository's config and current log file status.
 	 */
-	public ReflogWriter(Repository repository, boolean forceWrite) {
-		FS fs = repository.getFS();
-		parent = repository;
-		File gitDir = repository.getDirectory();
-		logsDir = fs.resolve(gitDir, LOGS);
-		logsRefsDir = fs.resolve(gitDir, LOGS + '/' + R_REFS);
+	public ReflogWriter(RefDirectory refdb, boolean forceWrite) {
+		this.refdb = refdb;
 		this.forceWrite = forceWrite;
 	}
 
 	/**
-	 * Get repository that reflog is being written for.
-	 *
-	 * @return file repository.
-	 */
-	public Repository getRepository() {
-		return parent;
-	}
-
-	/**
 	 * Create the log directories.
 	 *
 	 * @throws IOException
 	 * @return this writer.
 	 */
 	public ReflogWriter create() throws IOException {
-		FileUtils.mkdir(logsDir);
-		FileUtils.mkdir(logsRefsDir);
-		FileUtils.mkdir(new File(logsRefsDir,
-				R_HEADS.substring(R_REFS.length())));
+		FileUtils.mkdir(refdb.logsDir);
+		FileUtils.mkdir(refdb.logsRefsDir);
+		FileUtils.mkdir(
+				new File(refdb.logsRefsDir, R_HEADS.substring(R_REFS.length())));
 		return this;
 	}
 
 	/**
-	 * Locate the log file on disk for a single reference name.
-	 *
-	 * @param name
-	 *            name of the ref, relative to the Git repository top level
-	 *            directory (so typically starts with refs/).
-	 * @return the log file location.
-	 */
-	public File logFor(String name) {
-		if (name.startsWith(R_REFS)) {
-			name = name.substring(R_REFS.length());
-			return new File(logsRefsDir, name);
-		}
-		return new File(logsDir, name);
-	}
-
-	/**
 	 * Write the given entry to the ref's log.
 	 *
 	 * @param refName
@@ -208,7 +175,7 @@ public class ReflogWriter {
 
 		PersonIdent ident = update.getRefLogIdent();
 		if (ident == null)
-			ident = new PersonIdent(parent);
+			ident = new PersonIdent(refdb.getRepository());
 		else
 			ident = new PersonIdent(ident);
 
@@ -239,14 +206,14 @@ public class ReflogWriter {
 	}
 
 	private ReflogWriter log(String refName, byte[] rec) throws IOException {
-		File log = logFor(refName);
+		File log = refdb.logFor(refName);
 		boolean write = forceWrite
 				|| (isLogAllRefUpdates() && shouldAutoCreateLog(refName))
 				|| log.isFile();
 		if (!write)
 			return this;
 
-		WriteConfig wc = getRepository().getConfig().get(WriteConfig.KEY);
+		WriteConfig wc = refdb.getRepository().getConfig().get(WriteConfig.KEY);
 		FileOutputStream out;
 		try {
 			out = new FileOutputStream(log, true);
@@ -275,7 +242,8 @@ public class ReflogWriter {
 	}
 
 	private boolean isLogAllRefUpdates() {
-		return parent.getConfig().get(CoreConfig.KEY).isLogAllRefUpdates();
+		return refdb.getRepository().getConfig().get(CoreConfig.KEY)
+				.isLogAllRefUpdates();
 	}
 
 	private boolean shouldAutoCreateLog(String refName) {