From eadfcd3ec166c55c1ff3f3fe0b5e97dd94ff8d83 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 18 Jan 2016 10:42:00 -0800 Subject: [PATCH] ReceiveCommand.abort(): Utility to mark batch of commands as failed If one or more commands is failing the entire group usually has to also fail with "transaction aborted". Pull this loop into a helper so the idiom can be easily reused in several places throughout JGit. Change-Id: I3b9399b7e26ce2b0dc5f7baa85d585a433b4eaed --- .../storage/dfs/InMemoryRepository.java | 18 +++---------- .../internal/storage/reftree/Command.java | 26 +++++++++++++++++++ .../internal/storage/reftree/RefTree.java | 23 +++++----------- .../storage/reftree/RefTreeBatch.java | 26 +++---------------- .../jgit/transport/BaseReceivePack.java | 5 +--- .../jgit/transport/ReceiveCommand.java | 22 ++++++++++++++++ 6 files changed, 63 insertions(+), 57 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java index a050e1a5bd..5e246b47b4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java @@ -16,7 +16,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; -import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.storage.pack.PackExt; import org.eclipse.jgit.lib.BatchRefUpdate; import org.eclipse.jgit.lib.ObjectId; @@ -312,7 +311,7 @@ public class InMemoryRepository extends DfsRepository { try (RevWalk rw = new RevWalk(getRepository())) { for (ReceiveCommand c : cmds) { if (c.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) { - reject(cmds); + ReceiveCommand.abort(cmds); return; } @@ -324,7 +323,7 @@ public class InMemoryRepository extends DfsRepository { } } catch (IOException e) { c.setResult(ReceiveCommand.Result.REJECTED_MISSING_OBJECT); - reject(cmds); + ReceiveCommand.abort(cmds); return; } } @@ -337,7 +336,7 @@ public class InMemoryRepository extends DfsRepository { if (r == null) { if (c.getType() != ReceiveCommand.Type.CREATE) { c.setResult(ReceiveCommand.Result.LOCK_FAILURE); - reject(cmds); + ReceiveCommand.abort(cmds); return; } } else { @@ -345,7 +344,7 @@ public class InMemoryRepository extends DfsRepository { if (r.isSymbolic() || objectId == null || !objectId.equals(c.getOldId())) { c.setResult(ReceiveCommand.Result.LOCK_FAILURE); - reject(cmds); + ReceiveCommand.abort(cmds); return; } } @@ -374,15 +373,6 @@ public class InMemoryRepository extends DfsRepository { clearCache(); } - private void reject(List cmds) { - for (ReceiveCommand c : cmds) { - if (c.getResult() == ReceiveCommand.Result.NOT_ATTEMPTED) { - c.setResult(ReceiveCommand.Result.REJECTED_OTHER_REASON, - JGitText.get().transactionAborted); - } - } - } - @Override protected boolean compareAndPut(Ref oldRef, Ref newRef) throws IOException { diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java index 540c4384a7..dd08375f21 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/Command.java @@ -49,12 +49,14 @@ import static org.eclipse.jgit.lib.FileMode.TYPE_GITLINK; import static org.eclipse.jgit.lib.FileMode.TYPE_SYMLINK; import static org.eclipse.jgit.lib.Ref.Storage.NETWORK; import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; +import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON; import java.io.IOException; import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.dircache.DirCacheEntry; import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectIdRef; import org.eclipse.jgit.lib.ObjectInserter; @@ -79,6 +81,30 @@ import org.eclipse.jgit.transport.ReceiveCommand.Result; * for processing. */ public class Command { + /** + * Set unprocessed commands as failed due to transaction aborted. + *

+ * If a command is still {@link Result#NOT_ATTEMPTED} it will be set to + * {@link Result#REJECTED_OTHER_REASON}. If {@code why} is non-null its + * contents will be used as the message for the first command status. + * + * @param commands + * commands to mark as failed. + * @param why + * optional message to set on the first aborted command. + */ + public static void abort(Iterable commands, @Nullable String why) { + if (why == null || why.isEmpty()) { + why = JGitText.get().transactionAborted; + } + for (Command c : commands) { + if (c.getResult() == NOT_ATTEMPTED) { + c.setResult(REJECTED_OTHER_REASON, why); + why = JGitText.get().transactionAborted; + } + } + } + private final Ref oldRef; private final Ref newRef; private final ReceiveCommand cmd; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTree.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTree.java index 43eec5185b..85690c8ca5 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTree.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTree.java @@ -55,7 +55,6 @@ import static org.eclipse.jgit.lib.Ref.Storage.NEW; import static org.eclipse.jgit.lib.Ref.Storage.PACKED; import static org.eclipse.jgit.lib.RefDatabase.MAX_SYMBOLIC_REF_DEPTH; 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_OTHER_REASON; import java.io.IOException; @@ -248,7 +247,8 @@ public class RefTree { if (!isValidRef(cmd)) { cmd.setResult(REJECTED_OTHER_REASON, JGitText.get().funnyRefname); - return abort(cmdList); + Command.abort(cmdList, null); + return false; } apply(ed, cmd); } @@ -264,9 +264,11 @@ public class RefTree { break; } } - return abort(cmdList); + Command.abort(cmdList, null); + return false; } catch (LockFailureException e) { - return abort(cmdList); + Command.abort(cmdList, null); + return false; } } @@ -342,19 +344,6 @@ public class RefTree { } } - private static boolean abort(Iterable cmdList) { - for (Command cmd : cmdList) { - if (cmd.getResult() == NOT_ATTEMPTED) { - reject(cmd, JGitText.get().transactionAborted); - } - } - return false; - } - - private static void reject(Command cmd, String msg) { - cmd.setResult(REJECTED_OTHER_REASON, msg); - } - /** * Convert a path name in a RefTree to the reference name known by Git. * diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTreeBatch.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTreeBatch.java index 0cedea94d0..a55a9f51e7 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTreeBatch.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/reftree/RefTreeBatch.java @@ -98,7 +98,7 @@ class RefTreeBatch extends BatchRefUpdate { } if (c.getType() == UPDATE_NONFASTFORWARD) { c.setResult(REJECTED_NONFASTFORWARD); - reject(); + ReceiveCommand.abort(getCommands()); return; } } @@ -108,15 +108,6 @@ class RefTreeBatch extends BatchRefUpdate { execute(rw, todo); } - private void reject() { - String aborted = JGitText.get().transactionAborted; - for (ReceiveCommand c : getCommands()) { - if (c.getResult() == NOT_ATTEMPTED) { - c.setResult(REJECTED_OTHER_REASON, aborted); - } - } - } - void init(RevWalk rw) throws IOException { src = refdb.getBootstrap().exactRef(refdb.getTxnCommitted()); if (src != null && src.getObjectId() != null) { @@ -150,13 +141,13 @@ class RefTreeBatch extends BatchRefUpdate { void execute(RevWalk rw, List todo) throws IOException { for (Command c : todo) { if (c.getResult() != NOT_ATTEMPTED) { - reject(todo, JGitText.get().transactionAborted); + Command.abort(todo, null); return; } if (refdb.conflictsWithBootstrap(c.getRefName())) { c.setResult(REJECTED_OTHER_REASON, MessageFormat .format(JGitText.get().invalidRefName, c.getRefName())); - reject(todo, JGitText.get().transactionAborted); + Command.abort(todo, null); return; } } @@ -210,7 +201,7 @@ class RefTreeBatch extends BatchRefUpdate { c.setResult(OK); } } else { - reject(todo, commit.getResult().name()); + Command.abort(todo, commit.getResult().name()); } } @@ -228,13 +219,4 @@ class RefTreeBatch extends BatchRefUpdate { u.addCommand(commit); u.execute(rw, NullProgressMonitor.INSTANCE); } - - private static void reject(List todo, String msg) { - for (Command c : todo) { - if (c.getResult() == NOT_ATTEMPTED) { - c.setResult(REJECTED_OTHER_REASON, msg); - msg = JGitText.get().transactionAborted; - } - } - } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index 728643e925..a20e652553 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -1453,10 +1453,7 @@ public abstract class BaseReceivePack { * @since 3.6 */ protected void failPendingCommands() { - for (ReceiveCommand cmd : commands) { - if (cmd.getResult() == Result.NOT_ATTEMPTED) - cmd.setResult(Result.REJECTED_OTHER_REASON, JGitText.get().transactionAborted); - } + ReceiveCommand.abort(commands); } /** diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java index 5702b6d7b9..2b21c4a8fe 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java @@ -43,6 +43,9 @@ package org.eclipse.jgit.transport; +import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; +import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_OTHER_REASON; + import java.io.IOException; import java.text.MessageFormat; import java.util.ArrayList; @@ -168,6 +171,25 @@ public class ReceiveCommand { return filter((Iterable) commands, want); } + /** + * Set unprocessed commands as failed due to transaction aborted. + *

+ * If a command is still {@link Result#NOT_ATTEMPTED} it will be set to + * {@link Result#REJECTED_OTHER_REASON}. + * + * @param commands + * commands to mark as failed. + * @since 4.2 + */ + public static void abort(Iterable commands) { + for (ReceiveCommand c : commands) { + if (c.getResult() == NOT_ATTEMPTED) { + c.setResult(REJECTED_OTHER_REASON, + JGitText.get().transactionAborted); + } + } + } + private final ObjectId oldId; private final ObjectId newId; -- 2.39.5