Support symbolic references in ReceiveCommand

Allow creating symbolic references with link, and deleting them or
switching to ObjectId with unlink.  How this happens is up to the
individual RefDatabase.

The default implementation detaches RefUpdate if a symbolic reference
is involved, supporting these command instances on RefDirectory.
Unfortunately the packed-refs file does not support storing symrefs,
so atomic transactions involving more than one symref command are
failed early.

Updating InMemoryRepository is deferred until reftable lands, as I
plan to switch InMemoryRepository to use reftable for its internal
storage representation.

Change-Id: Ibcae068b17a2fc6d958f767f402a570ad88d9151
Signed-off-by: Minh Thai <mthai@google.com>
Signed-off-by: Terry Parker <tparker@google.com>

org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/storage/reftree/RefTreeDatabaseTest.java

@@ -657,7 +657,8 @@ public class RefTreeDatabaseTest {
 				Ref old = tree.exactRef(reader, name);
 				Command n;
 				try (RevWalk rw = new RevWalk(repo)) {
-					n = new Command(old, Command.toRef(rw, id, name, true));
+					n = new Command(old,
+							Command.toRef(rw, id, null, name, true));
 				}
 				return tree.apply(Collections.singleton(n));
 			}

org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@@ -22,6 +22,7 @@ atLeastOnePatternIsRequired=At least one pattern is required.
 atLeastTwoFiltersNeeded=At least two filters needed.
 atomicPushNotSupported=Atomic push not supported.
 atomicRefUpdatesNotSupported=Atomic ref updates not supported
+atomicSymRefNotSupported=Atomic symref not supported
 authenticationNotSupported=authentication not supported
 badBase64InputCharacterAt=Bad Base64 input character at {0} : {1} (decimal)
 badEntryDelimiter=Bad entry delimiter
@@ -41,6 +42,7 @@ blameNotCommittedYet=Not Committed Yet
 blobNotFound=Blob not found: {0}
 blobNotFoundForPath=Blob not found: {0} for path: {1}
 blockSizeNotPowerOf2=blockSize must be a power of 2
+bothRefTargetsMustNotBeNull=both old and new ref targets must not be null.
 branchNameInvalid=Branch name {0} is not allowed
 buildingBitmaps=Building bitmaps
 cachedPacksPreventsIndexCreation=Using cached packs prevents index creation
@@ -434,6 +436,7 @@ month=month
 months=months
 monthsAgo={0} months ago
 multipleMergeBasesFor=Multiple merge bases for:\n  {0}\n  {1} found:\n  {2}\n  {3}
+nameMustNotBeNullOrEmpty=Ref name must not be null or empty.
 need2Arguments=Need 2 arguments
 needPackOut=need packOut
 needsAtLeastOneEntry=Needs at least one entry

org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@@ -81,6 +81,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String atLeastTwoFiltersNeeded;
 	/***/ public String atomicPushNotSupported;
 	/***/ public String atomicRefUpdatesNotSupported;
+	/***/ public String atomicSymRefNotSupported;
 	/***/ public String authenticationNotSupported;
 	/***/ public String badBase64InputCharacterAt;
 	/***/ public String badEntryDelimiter;
@@ -100,6 +101,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String blobNotFound;
 	/***/ public String blobNotFoundForPath;
 	/***/ public String blockSizeNotPowerOf2;
+	/***/ public String bothRefTargetsMustNotBeNull;
 	/***/ public String branchNameInvalid;
 	/***/ public String buildingBitmaps;
 	/***/ public String cachedPacksPreventsIndexCreation;
@@ -493,6 +495,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String months;
 	/***/ public String monthsAgo;
 	/***/ public String multipleMergeBasesFor;
+	/***/ public String nameMustNotBeNullOrEmpty;
 	/***/ public String need2Arguments;
 	/***/ public String needPackOut;
 	/***/ public String needsAtLeastOneEntry;

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/file/PackedBatchRefUpdate.java

@@ -47,6 +47,7 @@ import static java.util.stream.Collectors.toList;
 import static org.eclipse.jgit.transport.ReceiveCommand.Result.LOCK_FAILURE;
 import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
 import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_NONFASTFORWARD;
+import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON;
 
 import java.io.IOException;
 import java.text.MessageFormat;
@@ -142,6 +143,12 @@ class PackedBatchRefUpdate extends BatchRefUpdate {
 			super.execute(walk, monitor, options);
 			return;
 		}
+		if (containsSymrefs(pending)) {
+			// packed-refs file cannot store symrefs
+			reject(pending.get(0), REJECTED_OTHER_REASON,
+					JGitText.get().atomicSymRefNotSupported, pending);
+			return;
+		}
 
 		// Required implementation details copied from super.execute.
 		if (!blockUntilTimestamps(MAX_WAIT)) {
@@ -209,6 +216,15 @@ class PackedBatchRefUpdate extends BatchRefUpdate {
 		writeReflog(pending);
 	}
 
+	private static boolean containsSymrefs(List<ReceiveCommand> commands) {
+		for (ReceiveCommand cmd : commands) {
+			if (cmd.getOldSymref() != null || cmd.getNewSymref() != null) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	private boolean checkConflictingNames(List<ReceiveCommand> commands)
 			throws IOException {
 		Set<String> takenNames = new HashSet<>();
@@ -510,7 +526,12 @@ class PackedBatchRefUpdate extends BatchRefUpdate {
 
 	private static void reject(ReceiveCommand cmd, ReceiveCommand.Result result,
 			List<ReceiveCommand> commands) {
-		cmd.setResult(result);
+		reject(cmd, result, null, commands);
+	}
+
+	private static void reject(ReceiveCommand cmd, ReceiveCommand.Result result,
+			String why, List<ReceiveCommand> commands) {
+		cmd.setResult(result, why);
 		for (ReceiveCommand c2 : commands) {
 			if (c2.getResult() == ReceiveCommand.Result.OK) {
 				// Undo OK status so ReceiveCommand#abort aborts it. Assumes this method

org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java

@@ -61,6 +61,7 @@ import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdRef;
 import org.eclipse.jgit.lib.ObjectInserter;
 import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.SymbolicRef;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevTag;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -153,14 +154,20 @@ public class Command {
 	 */
 	public Command(RevWalk rw, ReceiveCommand cmd)
 			throws MissingObjectException, IOException {
-		this.oldRef = toRef(rw, cmd.getOldId(), cmd.getRefName(), false);
+		this.oldRef = toRef(rw, cmd.getOldId(), cmd.getOldSymref(),
-		this.newRef = toRef(rw, cmd.getNewId(), cmd.getRefName(), true);
+				cmd.getRefName(), false);
+		this.newRef = toRef(rw, cmd.getNewId(), cmd.getNewSymref(),
+				cmd.getRefName(), true);
 		this.cmd = cmd;
 	}
 
-	static Ref toRef(RevWalk rw, ObjectId id, String name,
+	static Ref toRef(RevWalk rw, ObjectId id, @Nullable String target,
-			boolean mustExist) throws MissingObjectException, IOException {
+			String name, boolean mustExist)
-		if (ObjectId.zeroId().equals(id)) {
+			throws MissingObjectException, IOException {
+		if (target != null) {
+			return new SymbolicRef(name,
+					new ObjectIdRef.Unpeeled(NETWORK, target, id));
+		} else if (ObjectId.zeroId().equals(id)) {
 			return null;
 		}
 

org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java

@@ -52,6 +52,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 
+import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.AnyObjectId;
@@ -196,8 +197,8 @@ public class ReceiveCommand {
 	 *
 	 * @param cmd
 	 *            command.
-	 * @return whether the command failed due to transaction aborted, as in {@link
+	 * @return whether the command failed due to transaction aborted, as in
-	 *         #abort(Iterable)}.
+	 *         {@link #abort(Iterable)}.
 	 * @since 4.9
 	 */
 	public static boolean isTransactionAborted(ReceiveCommand cmd) {
@@ -205,14 +206,71 @@ public class ReceiveCommand {
 				&& cmd.getMessage().equals(JGitText.get().transactionAborted);
 	}
 
+	/**
+	 * Create a command to switch a reference from object to symbolic.
+	 *
+	 * @param oldId
+	 *            expected oldId. May be {@code zeroId} to create.
+	 * @param newTarget
+	 *            new target; must begin with {@code "refs/"}.
+	 * @param name
+	 *            name of the reference to make symbolic.
+	 * @return command instance.
+	 * @since 4.10
+	 */
+	public static ReceiveCommand link(@NonNull ObjectId oldId,
+			@NonNull String newTarget, @NonNull String name) {
+		return new ReceiveCommand(oldId, newTarget, name);
+	}
+
+	/**
+	 * Create a command to switch a symbolic reference's target.
+	 *
+	 * @param oldTarget
+	 *            expected old target. May be null to create.
+	 * @param newTarget
+	 *            new target; must begin with {@code "refs/"}.
+	 * @param name
+	 *            name of the reference to make symbolic.
+	 * @return command instance.
+	 * @since 4.10
+	 */
+	public static ReceiveCommand link(@Nullable String oldTarget,
+			@NonNull String newTarget, @NonNull String name) {
+		return new ReceiveCommand(oldTarget, newTarget, name);
+	}
+
+	/**
+	 * Create a command to switch a reference from symbolic to object.
+	 *
+	 * @param oldTarget
+	 *            expected old target.
+	 * @param newId
+	 *            new object identifier. May be {@code zeroId()} to delete.
+	 * @param name
+	 *            name of the reference to convert from symbolic.
+	 * @return command instance.
+	 * @since 4.10
+	 */
+	public static ReceiveCommand unlink(@NonNull String oldTarget,
+			@NonNull ObjectId newId, @NonNull String name) {
+		return new ReceiveCommand(oldTarget, newId, name);
+	}
+
 	private final ObjectId oldId;
 
+	private final String oldSymref;
+
 	private final ObjectId newId;
 
+	private final String newSymref;
+
 	private final String name;
 
 	private Type type;
 
+	private boolean typeIsCorrect;
+
 	private Ref ref;
 
 	private Result status = Result.NOT_ATTEMPTED;
@@ -227,8 +285,6 @@ public class ReceiveCommand {
 
 	private Boolean forceRefLog;
 
-	private boolean typeIsCorrect;
-
 	/**
 	 * Create a new command for {@link BaseReceivePack}.
 	 *
@@ -244,13 +300,21 @@ public class ReceiveCommand {
 	public ReceiveCommand(final ObjectId oldId, final ObjectId newId,
 			final String name) {
 		if (oldId == null) {
-			throw new IllegalArgumentException(JGitText.get().oldIdMustNotBeNull);
+			throw new IllegalArgumentException(
+					JGitText.get().oldIdMustNotBeNull);
 		}
 		if (newId == null) {
-			throw new IllegalArgumentException(JGitText.get().newIdMustNotBeNull);
+			throw new IllegalArgumentException(
+					JGitText.get().newIdMustNotBeNull);
+		}
+		if (name == null || name.isEmpty()) {
+			throw new IllegalArgumentException(
+					JGitText.get().nameMustNotBeNullOrEmpty);
 		}
 		this.oldId = oldId;
+		this.oldSymref = null;
 		this.newId = newId;
+		this.newSymref = null;
 		this.name = name;
 
 		type = Type.UPDATE;
@@ -275,19 +339,28 @@ public class ReceiveCommand {
 	 *            name of the ref being affected.
 	 * @param type
 	 *            type of the command. Must be {@link Type#CREATE} if {@code
-	 *            oldId} is zero, or {@link Type#DELETE} if {@code newId} is zero.
+	 *            oldId} is zero, or {@link Type#DELETE} if {@code newId} is
+	 *            zero.
 	 * @since 2.0
 	 */
 	public ReceiveCommand(final ObjectId oldId, final ObjectId newId,
 			final String name, final Type type) {
 		if (oldId == null) {
-			throw new IllegalArgumentException(JGitText.get().oldIdMustNotBeNull);
+			throw new IllegalArgumentException(
+					JGitText.get().oldIdMustNotBeNull);
 		}
 		if (newId == null) {
-			throw new IllegalArgumentException(JGitText.get().newIdMustNotBeNull);
+			throw new IllegalArgumentException(
+					JGitText.get().newIdMustNotBeNull);
+		}
+		if (name == null || name.isEmpty()) {
+			throw new IllegalArgumentException(
+					JGitText.get().nameMustNotBeNullOrEmpty);
 		}
 		this.oldId = oldId;
+		this.oldSymref = null;
 		this.newId = newId;
+		this.newSymref = null;
 		this.name = name;
 		switch (type) {
 		case CREATE:
@@ -311,21 +384,144 @@ public class ReceiveCommand {
 			}
 			break;
 		default:
-			throw new IllegalStateException(JGitText.get().enumValueNotSupported0);
+			throw new IllegalStateException(
+					JGitText.get().enumValueNotSupported0);
 		}
 		this.type = type;
 	}
 
+	/**
+	 * Create a command to switch a reference from object to symbolic.
+	 *
+	 * @param oldId
+	 *            the old object id; must not be null. Use
+	 *            {@link ObjectId#zeroId()} to indicate a ref creation.
+	 * @param newSymref
+	 *            new target, must begin with {@code "refs/"}. Use {@code null}
+	 *            to indicate a ref deletion.
+	 * @param name
+	 *            name of the reference to make symbolic.
+	 * @since 4.10
+	 */
+	private ReceiveCommand(ObjectId oldId, String newSymref, String name) {
+		if (oldId == null) {
+			throw new IllegalArgumentException(
+					JGitText.get().oldIdMustNotBeNull);
+		}
+		if (name == null || name.isEmpty()) {
+			throw new IllegalArgumentException(
+					JGitText.get().nameMustNotBeNullOrEmpty);
+		}
+		this.oldId = oldId;
+		this.oldSymref = null;
+		this.newId = ObjectId.zeroId();
+		this.newSymref = newSymref;
+		this.name = name;
+		if (AnyObjectId.equals(ObjectId.zeroId(), oldId)) {
+			type = Type.CREATE;
+		} else if (newSymref != null) {
+			type = Type.UPDATE;
+		} else {
+			type = Type.DELETE;
+		}
+		typeIsCorrect = true;
+	}
+
+	/**
+	 * Create a command to switch a reference from symbolic to object.
+	 *
+	 * @param oldSymref
+	 *            expected old target. Use {@code null} to indicate a ref
+	 *            creation.
+	 * @param newId
+	 *            the new object id; must not be null. Use
+	 *            {@link ObjectId#zeroId()} to indicate a ref deletion.
+	 * @param name
+	 *            name of the reference to convert from symbolic.
+	 * @since 4.10
+	 */
+	private ReceiveCommand(String oldSymref, ObjectId newId, String name) {
+		if (newId == null) {
+			throw new IllegalArgumentException(
+					JGitText.get().newIdMustNotBeNull);
+		}
+		if (name == null || name.isEmpty()) {
+			throw new IllegalArgumentException(
+					JGitText.get().nameMustNotBeNullOrEmpty);
+		}
+		this.oldId = ObjectId.zeroId();
+		this.oldSymref = oldSymref;
+		this.newId = newId;
+		this.newSymref = null;
+		this.name = name;
+		if (oldSymref == null) {
+			type = Type.CREATE;
+		} else if (!AnyObjectId.equals(ObjectId.zeroId(), newId)) {
+			type = Type.UPDATE;
+		} else {
+			type = Type.DELETE;
+		}
+		typeIsCorrect = true;
+	}
+
+	/**
+	 * Create a command to switch a symbolic reference's target.
+	 *
+	 * @param oldTarget
+	 *            expected old target. Use {@code null} to indicate a ref
+	 *            creation.
+	 * @param newTarget
+	 *            new target. Use {@code null} to indicate a ref deletion.
+	 * @param name
+	 *            name of the reference to make symbolic.
+	 * @since 4.10
+	 */
+	private ReceiveCommand(@Nullable String oldTarget, String newTarget, String name) {
+		if (name == null || name.isEmpty()) {
+			throw new IllegalArgumentException(
+					JGitText.get().nameMustNotBeNullOrEmpty);
+		}
+		this.oldId = ObjectId.zeroId();
+		this.oldSymref = oldTarget;
+		this.newId = ObjectId.zeroId();
+		this.newSymref = newTarget;
+		this.name = name;
+		if (oldTarget == null) {
+			if (newTarget == null) {
+				throw new IllegalArgumentException(
+						JGitText.get().bothRefTargetsMustNotBeNull);
+			}
+			type = Type.CREATE;
+		} else if (newTarget != null) {
+			type = Type.UPDATE;
+		} else {
+			type = Type.DELETE;
+		}
+		typeIsCorrect = true;
+	}
+
 	/** @return the old value the client thinks the ref has. */
 	public ObjectId getOldId() {
 		return oldId;
 	}
 
+	/** @return expected old target for a symbolic reference. */
+	@Nullable
+	public String getOldSymref() {
+		return oldSymref;
+	}
+
 	/** @return the requested new value for this ref. */
 	public ObjectId getNewId() {
 		return newId;
 	}
 
+	/** @return requested new target for a symbolic reference. */
+	@Nullable
+	public String getNewSymref() {
+		return newSymref;
+	}
+
 	/** @return the name of the ref being updated. */
 	public String getRefName() {
 		return name;
@@ -452,8 +648,8 @@ public class ReceiveCommand {
 	/**
 	 * Check whether the reflog should be written regardless of repo defaults.
 	 *
-	 * @return whether force writing is enabled; null if {@code
+	 * @return whether force writing is enabled; {@code null} if
-	 * #setForceRefLog(boolean)} was never called.
+	 *         {@code #setForceRefLog(boolean)} was never called.
 	 * @since 4.9
 	 */
 	@Nullable
@@ -525,7 +721,18 @@ public class ReceiveCommand {
 	 */
 	public void execute(final BaseReceivePack rp) {
 		try {
-			final RefUpdate ru = rp.getRepository().updateRef(getRefName());
+			String expTarget = getOldSymref();
+			boolean detach = getNewSymref() != null
+					|| (type == Type.DELETE && expTarget != null);
+			RefUpdate ru = rp.getRepository().updateRef(getRefName(), detach);
+			if (expTarget != null) {
+				if (!ru.getRef().isSymbolic() || !ru.getRef().getTarget()
+						.getName().equals(expTarget)) {
+					setResult(Result.LOCK_FAILURE);
+					return;
+				}
+			}
+
 			ru.setRefLogIdent(rp.getRefLogIdent());
 			ru.setRefLogMessage(refLogMessage, refLogIncludeResult);
 			switch (getType()) {
@@ -546,9 +753,13 @@ public class ReceiveCommand {
 			case UPDATE_NONFASTFORWARD:
 				ru.setForceUpdate(rp.isAllowNonFastForwards());
 				ru.setExpectedOldObjectId(getOldId());
-				ru.setNewObjectId(getNewId());
 				ru.setRefLogMessage("push", true); //$NON-NLS-1$
-				setResult(ru.update(rp.getRevWalk()));
+				if (getNewSymref() != null) {
+					setResult(ru.link(getNewSymref()));
+				} else {
+					ru.setNewObjectId(getNewId());
+					setResult(ru.update(rp.getRevWalk()));
+				}
 				break;
 			}
 		} catch (IOException err) {