Replace writeSymref with RefUpdate.link

By using RefUpdate for symbolic reference creation we can reuse
the logic related to updating the reflog with the event, without
needing to expose something such as the legacy ReflogWriter class
(which we no longer have).

Applications using writeSymref must update their code to use the
new pattern of changing the reference through the updateRef method:

    String refName = "refs/heads/master";
    RefUpdate u = repository.updateRef(Constants.HEAD);
    u.setRefLogMessage("checkout: moving to " + refName, false);
    switch (u.link(refName)) {
    case NEW:
    case FORCED:
    case NO_CHANGE:
        // A successful update of the reference
        break;
    default:
        // Handle the failure, e.g. for older behavior
        throw new IOException(u.getResult());
    }

Change-Id: I1093e1ec2970147978a786cfdd0a75d0aebf8010
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

9 files changed, 147 insertions, 67 deletions

diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java
index 3fe50d6682..571f34b05d 100644
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Clone.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2009, Google Inc.
+ * Copyright (C) 2008-2010, Google Inc.
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -164,8 +164,11 @@ class Clone extends AbstractFetchCommand {
 	private void doCheckout(final Ref branch) throws IOException {
 		if (branch == null)
 			throw die("cannot checkout; no HEAD advertised by remote");
-		if (!Constants.HEAD.equals(branch.getName()))
-			db.writeSymref(Constants.HEAD, branch.getName());
+		if (!Constants.HEAD.equals(branch.getName())) {
+			RefUpdate u = db.updateRef(Constants.HEAD);
+			u.disableRefLog();
+			u.link(branch.getName());
+		}
 
 		final Commit commit = db.mapCommit(branch.getObjectId());
 		final RefUpdate u = db.updateRef(Constants.HEAD);
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java
index 42c8a92b93..100b758e65 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefTest.java
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2009-2010, Google Inc.
  * Copyright (C) 2009, Robin Rosenberg
  * Copyright (C) 2009, Robin Rosenberg <robin.rosenberg@dewire.com>
  * and other copyright owners as documented in the project's IP log.
@@ -58,15 +59,26 @@ import org.eclipse.jgit.lib.RefUpdate.Result;
  */
 public class RefTest extends SampleDataRepositoryTestCase {
 
+	private void writeSymref(String src, String dst) throws IOException {
+		RefUpdate u = db.updateRef(src);
+		switch (u.link(dst)) {
+		case NEW:
+		case FORCED:
+		case NO_CHANGE:
+			break;
+		default:
+			fail("link " + src + " to " + dst);
+		}
+	}
+
 	public void testReadAllIncludingSymrefs() throws Exception {
 		ObjectId masterId = db.resolve("refs/heads/master");
 		RefUpdate updateRef = db.updateRef("refs/remotes/origin/master");
 		updateRef.setNewObjectId(masterId);
 		updateRef.setForceUpdate(true);
 		updateRef.update();
-		db
-				.writeSymref("refs/remotes/origin/HEAD",
-						"refs/remotes/origin/master");
+		writeSymref("refs/remotes/origin/HEAD",
+					"refs/remotes/origin/master");
 
 		ObjectId r = db.resolve("refs/remotes/origin/HEAD");
 		assertEquals(masterId, r);
@@ -85,7 +97,7 @@ public class RefTest extends SampleDataRepositoryTestCase {
 	}
 
 	public void testReadSymRefToPacked() throws IOException {
-		db.writeSymref("HEAD", "refs/heads/b");
+		writeSymref("HEAD", "refs/heads/b");
 		Ref ref = db.getRef("HEAD");
 		assertEquals(Ref.Storage.LOOSE, ref.getStorage());
 		assertTrue("is symref", ref.isSymbolic());
@@ -102,7 +114,7 @@ public class RefTest extends SampleDataRepositoryTestCase {
 		Result update = updateRef.update();
 		assertEquals(Result.FORCED, update); // internal
 
-		db.writeSymref("HEAD", "refs/heads/master");
+		writeSymref("HEAD", "refs/heads/master");
 		Ref ref = db.getRef("HEAD");
 		assertEquals(Ref.Storage.LOOSE, ref.getStorage());
 		ref = ref.getTarget();
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java
index cb9111758a..c8a5b33f68 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/RefUpdateTest.java
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2008, Charles O'Farrell <charleso@charleso.org>
+ * Copyright (C) 2009-2010, Google Inc.
  * Copyright (C) 2008-2009, Robin Rosenberg <robin.rosenberg@dewire.com>
  * and other copyright owners as documented in the project's IP log.
  *
@@ -56,6 +57,18 @@ import org.eclipse.jgit.revwalk.RevWalk;
 
 public class RefUpdateTest extends SampleDataRepositoryTestCase {
 
+	private void writeSymref(String src, String dst) throws IOException {
+		RefUpdate u = db.updateRef(src);
+		switch (u.link(dst)) {
+		case NEW:
+		case FORCED:
+		case NO_CHANGE:
+			break;
+		default:
+			fail("link " + src + " to " + dst);
+		}
+	}
+
 	private RefUpdate updateRef(final String name) throws IOException {
 		final RefUpdate ref = db.updateRef(name);
 		ref.setNewObjectId(db.resolve(Constants.HEAD));
@@ -328,7 +341,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 	 */
 	public void testUpdateRefDetachedUnbornHead() throws Exception {
 		ObjectId ppid = db.resolve("refs/heads/master^");
-		db.writeSymref("HEAD", "refs/heads/unborn");
+		writeSymref("HEAD", "refs/heads/unborn");
 		RefUpdate updateRef = db.updateRef("HEAD", true);
 		updateRef.setForceUpdate(true);
 		updateRef.setNewObjectId(ppid);
@@ -437,7 +450,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 		// Do not use the defalt repo for this case.
 		Map<String, Ref> allRefs = db.getAllRefs();
 		ObjectId oldValue = db.resolve("HEAD");
-		db.writeSymref(Constants.HEAD, "refs/heads/newref");
+		writeSymref(Constants.HEAD, "refs/heads/newref");
 		RefUpdate updateRef = db.updateRef(Constants.HEAD);
 		updateRef.setForceUpdate(true);
 		updateRef.setNewObjectId(oldValue);
@@ -601,7 +614,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 
 	public void testRenameCurrentBranch() throws IOException {
 		ObjectId rb = db.resolve("refs/heads/b");
-		db.writeSymref(Constants.HEAD, "refs/heads/b");
+		writeSymref(Constants.HEAD, "refs/heads/b");
 		ObjectId oldHead = db.resolve(Constants.HEAD);
 		assertTrue("internal test condition, b == HEAD", rb.equals(oldHead));
 		writeReflog(db, rb, rb, "Just a message", "refs/heads/b");
@@ -659,7 +672,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 	public void tryRenameWhenLocked(String toLock, String fromName,
 			String toName, String headPointsTo) throws IOException {
 		// setup
-		db.writeSymref(Constants.HEAD, headPointsTo);
+		writeSymref(Constants.HEAD, headPointsTo);
 		ObjectId oldfromId = db.resolve(fromName);
 		ObjectId oldHeadId = db.resolve(Constants.HEAD);
 		writeReflog(db, oldfromId, oldfromId, "Just a message",
@@ -753,7 +766,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 	public void testRenameRefNameColission1avoided() throws IOException {
 		// setup
 		ObjectId rb = db.resolve("refs/heads/b");
-		db.writeSymref(Constants.HEAD, "refs/heads/a");
+		writeSymref(Constants.HEAD, "refs/heads/a");
 		RefUpdate updateRef = db.updateRef("refs/heads/a");
 		updateRef.setNewObjectId(rb);
 		updateRef.setRefLogMessage("Setup", false);
@@ -785,7 +798,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 	public void testRenameRefNameColission2avoided() throws IOException {
 		// setup
 		ObjectId rb = db.resolve("refs/heads/b");
-		db.writeSymref(Constants.HEAD, "refs/heads/prefix/a");
+		writeSymref(Constants.HEAD, "refs/heads/prefix/a");
 		RefUpdate updateRef = db.updateRef("refs/heads/prefix/a");
 		updateRef.setNewObjectId(rb);
 		updateRef.setRefLogMessage("Setup", false);
@@ -823,6 +836,6 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
 		RefDirectoryUpdate update = refs.newUpdate(refName, true);
 		update.setOldObjectId(oldId);
 		update.setNewObjectId(newId);
-		refs.log(update, msg);
+		refs.log(update, msg, true);
 	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java
index 22d0c1ad3b..21e7041b32 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDatabase.java
@@ -114,19 +114,6 @@ public abstract class RefDatabase {
 	public abstract boolean isNameConflicting(String name) throws IOException;
 
 	/**
-	 * Create a symbolic reference from one name to another.
-	 *
-	 * @param name
-	 *            the name of the reference. Should be {@link Constants#HEAD} or
-	 *            starting with {@link Constants#R_REFS}.
-	 * @param target
-	 *            the target of the reference.
-	 * @throws IOException
-	 *             the reference could not be created or overwritten.
-	 */
-	public abstract void link(String name, String target) throws IOException;
-
-	/**
 	 * Create a new update command to create, modify or delete a reference.
 	 *
 	 * @param name
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java
index 9fdb48fe83..90ac0bf47e 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectory.java
@@ -439,24 +439,11 @@ public class RefDirectory extends RefDatabase {
 		return leaf;
 	}
 
-	@Override
-	public void link(String name, String target) throws IOException {
-		LockFile lck = new LockFile(fileFor(name));
-		if (!lck.lock())
-			throw new IOException("Cannot lock " + name);
-		lck.setNeedStatInformation(true);
-		try {
-			lck.write(encode(SYMREF + target + '\n'));
-			if (!lck.commit())
-				throw new IOException("Cannot write " + name);
-		} finally {
-			lck.unlock();
-		}
-		putLooseRef(newSymbolicRef(lck.getCommitLastModified(), name, target));
+	void storedSymbolicRef(RefDirectoryUpdate u, long modified, String target) {
+		putLooseRef(newSymbolicRef(modified, u.getRef().getName(), target));
 		fireRefsChanged();
 	}
 
-	@Override
 	public RefDirectoryUpdate newUpdate(String name, boolean detach)
 			throws IOException {
 		final RefList<Ref> packed = getPackedRefs();
@@ -536,7 +523,8 @@ public class RefDirectory extends RefDatabase {
 		fireRefsChanged();
 	}
 
-	void log(final RefUpdate update, final String msg) throws IOException {
+	void log(final RefUpdate update, final String msg, final boolean deref)
+			throws IOException {
 		final ObjectId oldId = update.getOldObjectId();
 		final ObjectId newId = update.getNewObjectId();
 		final Ref ref = update.getRef();
@@ -558,9 +546,12 @@ public class RefDirectory extends RefDatabase {
 		r.append('\n');
 		final byte[] rec = encode(r.toString());
 
-		if (ref.isSymbolic())
+		if (deref && ref.isSymbolic()) {
+			log(ref.getName(), rec);
+			log(ref.getLeaf().getName(), rec);
+		} else {
 			log(ref.getName(), rec);
-		log(ref.getLeaf().getName(), rec);
+		}
 	}
 
 	private void log(final String refName, final byte[] rec) throws IOException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java
index e43d2a5617..fec00d9325 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryRename.java
@@ -208,8 +208,16 @@ class RefDirectoryRename extends RefRename {
 
 	private boolean linkHEAD(RefUpdate target) {
 		try {
-			refdb.link(Constants.HEAD, target.getName());
-			return true;
+			RefUpdate u = refdb.newUpdate(Constants.HEAD, false);
+			u.disableRefLog();
+			switch (u.link(target.getName())) {
+			case NEW:
+			case FORCED:
+			case NO_CHANGE:
+				return true;
+			default:
+				return false;
+			}
 		} catch (IOException e) {
 			return false;
 		}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java
index 113b74b44e..447be104ab 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefDirectoryUpdate.java
@@ -44,6 +44,8 @@
 
 package org.eclipse.jgit.lib;
 
+import static org.eclipse.jgit.lib.Constants.encode;
+
 import java.io.IOException;
 
 /** Updates any reference stored by {@link RefDirectory}. */
@@ -68,8 +70,10 @@ class RefDirectoryUpdate extends RefUpdate {
 	}
 
 	@Override
-	protected boolean tryLock() throws IOException {
-		Ref dst = getRef().getLeaf();
+	protected boolean tryLock(boolean deref) throws IOException {
+		Ref dst = getRef();
+		if (deref)
+			dst = dst.getLeaf();
 		String name = dst.getName();
 		lock = new LockFile(database.fileFor(name));
 		if (lock.lock()) {
@@ -105,7 +109,7 @@ class RefDirectoryUpdate extends RefUpdate {
 						msg = strResult;
 				}
 			}
-			database.log(this, msg);
+			database.log(this, msg, true);
 		}
 		if (!lock.commit())
 			return Result.LOCK_FAILURE;
@@ -132,4 +136,21 @@ class RefDirectoryUpdate extends RefUpdate {
 			database.delete(this);
 		return status;
 	}
+
+	@Override
+	protected Result doLink(final String target) throws IOException {
+		lock.setNeedStatInformation(true);
+		lock.write(encode(RefDirectory.SYMREF + target + '\n'));
+
+		String msg = getRefLogMessage();
+		if (msg != null)
+			database.log(this, msg, false);
+		if (!lock.commit())
+			return Result.LOCK_FAILURE;
+		database.storedSymbolicRef(this, lock.getCommitLastModified(), target);
+
+		if (getRef().getStorage() == Ref.Storage.NEW)
+			return Result.NEW;
+		return Result.FORCED;
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
index 33e12a110d..553266284b 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java
@@ -183,13 +183,17 @@ public abstract class RefUpdate {
 	 * If the locking was successful the implementor must set the current
 	 * identity value by calling {@link #setOldObjectId(ObjectId)}.
 	 *
+	 * @param deref
+	 *            true if the lock should be taken against the leaf level
+	 *            reference; false if it should be taken exactly against the
+	 *            current reference.
 	 * @return true if the lock was acquired and the reference is likely
 	 *         protected from concurrent modification; false if it failed.
 	 * @throws IOException
 	 *             the lock couldn't be taken due to an unexpected storage
 	 *             failure, and not because of a concurrent update.
 	 */
-	protected abstract boolean tryLock() throws IOException;
+	protected abstract boolean tryLock(boolean deref) throws IOException;
 
 	/** Releases the lock taken by {@link #tryLock} if it succeeded. */
 	protected abstract void unlock();
@@ -209,6 +213,13 @@ public abstract class RefUpdate {
 	protected abstract Result doDelete(Result desiredResult) throws IOException;
 
 	/**
+	 * @param target
+	 * @return {@link Result#NEW} on success.
+	 * @throws IOException
+	 */
+	protected abstract Result doLink(String target) throws IOException;
+
+	/**
 	 * Get the name of the ref this update will operate on.
 	 *
 	 * @return name of underlying ref.
@@ -499,6 +510,50 @@ public abstract class RefUpdate {
 		}
 	}
 
+	/**
+	 * Replace this reference with a symbolic reference to another reference.
+	 * <p>
+	 * This exact reference (not its traversed leaf) is replaced with a symbolic
+	 * reference to the requested name.
+	 *
+	 * @param target
+	 *            name of the new target for this reference. The new target name
+	 *            must be absolute, so it must begin with {@code refs/}.
+	 * @return {@link Result#NEW} or {@link Result#FORCED} on success.
+	 * @throws IOException
+	 */
+	public Result link(String target) throws IOException {
+		if (!target.startsWith(Constants.R_REFS))
+			throw new IllegalArgumentException("Not " + Constants.R_REFS);
+		if (getRefDatabase().isNameConflicting(getName()))
+			return Result.LOCK_FAILURE;
+		try {
+			if (!tryLock(false))
+				return Result.LOCK_FAILURE;
+
+			final Ref old = getRefDatabase().getRef(getName());
+			if (old != null && old.isSymbolic()) {
+				final Ref dst = old.getTarget();
+				if (target.equals(dst.getName()))
+					return result = Result.NO_CHANGE;
+			}
+
+			if (old != null && old.getObjectId() != null)
+				setOldObjectId(old.getObjectId());
+
+			final Ref dst = getRefDatabase().getRef(target);
+			if (dst != null && dst.getObjectId() != null)
+				setNewObjectId(dst.getObjectId());
+
+			return result = doLink(target);
+		} catch (IOException x) {
+			result = Result.IO_FAILURE;
+			throw x;
+		} finally {
+			unlock();
+		}
+	}
+
 	private Result updateImpl(final RevWalk walk, final Store store)
 			throws IOException {
 		RevObject newObj;
@@ -507,7 +562,7 @@ public abstract class RefUpdate {
 		if (getRefDatabase().isNameConflicting(getName()))
 			return Result.LOCK_FAILURE;
 		try {
-			if (!tryLock())
+			if (!tryLock(true))
 				return Result.LOCK_FAILURE;
 			if (expValue != null) {
 				final ObjectId o;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
index ecae243a41..ca86e36a47 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Repository.java
@@ -276,8 +276,10 @@ public class Repository {
 		objectDatabase.create();
 
 		new File(gitDir, "branches").mkdir();
-		final String master = Constants.R_HEADS + Constants.MASTER;
-		refs.link(Constants.HEAD, master);
+
+		RefUpdate head = updateRef(Constants.HEAD);
+		head.disableRefLog();
+		head.link(Constants.R_HEADS + Constants.MASTER);
 
 		cfg.setInt("core", null, "repositoryformatversion", 0);
 		cfg.setBoolean("core", null, "filemode", true);
@@ -899,18 +901,6 @@ public class Repository {
 		objectDatabase.openPack(pack, idx);
 	}
 
-    /**
-     * Writes a symref (e.g. HEAD) to disk
-     *
-     * @param name symref name
-     * @param target pointed to ref
-     * @throws IOException
-     */
-    public void writeSymref(final String name, final String target)
-			throws IOException {
-		refs.link(name, target);
-	}
-
 	public String toString() {
 		return "Repository[" + getDirectory() + "]";
 	}