From 5fb5bd7ab0318c4f2e58d1d994769056a1121725 Mon Sep 17 00:00:00 2001 From: Marek Zawirski Date: Sun, 17 Aug 2008 22:43:52 +0200 Subject: [PATCH] Clean up exception issues in RemoteRefUpdate IllegalArgumentException was probably a wrong choice for exception thrown in RemoteRefUpdate when src object can't be resolved - it should be checked one. Now it's IOException, which makes call more safe for external clients. Signed-off-by: Marek Zawirski Signed-off-by: Robin Rosenberg --- .../spearce/jgit/transport/RemoteRefUpdate.java | 12 +++--- .../src/org/spearce/jgit/transport/Transport.java | 46 +++++++++++----------- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteRefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteRefUpdate.java index 7db6c55c..5afb8a46 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteRefUpdate.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteRefUpdate.java @@ -166,23 +166,23 @@ public class RemoteRefUpdate { * remote ref with this name. * @throws IOException * when I/O error occurred during creating - * {@link TrackingRefUpdate} for local tracking branch. + * {@link TrackingRefUpdate} for local tracking branch or srcRef + * can't be resolved to any object. * @throws IllegalArgumentException - * if some required parameter was null or srcRef can't be - * resolved to any object. + * if some required parameter was null */ public RemoteRefUpdate(final Repository db, final String srcRef, final String remoteName, final boolean forceUpdate, final String localName, final ObjectId expectedOldObjectId) throws IOException { if (remoteName == null) - throw new IllegalArgumentException("remote name can't be null"); + throw new IllegalArgumentException("Remote name can't be null."); this.srcRef = srcRef; this.newObjectId = (srcRef == null ? ObjectId.zeroId() : db .resolve(srcRef)); if (newObjectId == null) - throw new IllegalArgumentException( - "source ref doesn't resolve to any object"); + throw new IOException("Source ref " + srcRef + + " doesn't resolve to any object."); this.remoteName = remoteName; this.forceUpdate = forceUpdate; if (localName != null && db != null) diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java index 14a03493..862ba9f4 100644 --- a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java @@ -39,6 +39,7 @@ package org.spearce.jgit.transport; +import java.io.IOException; import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Collection; @@ -228,38 +229,29 @@ public abstract class Transport { * fetch specifications used for finding localtracking refs. May * be null or empty collection. * @return collection of set up {@link RemoteRefUpdate}. - * @throws TransportException + * @throws IOException * when problem occurred during conversion or specification set * up: most probably, missing objects or refs. */ public static Collection findRemoteRefUpdatesFor( final Repository db, final Collection specs, - Collection fetchSpecs) throws TransportException { + Collection fetchSpecs) throws IOException { if (fetchSpecs == null) fetchSpecs = Collections.emptyList(); final List result = new LinkedList(); final Collection procRefs = expandPushWildcardsFor(db, specs); for (final RefSpec spec : procRefs) { - try { - final String srcRef = spec.getSource(); - // null destination (no-colon in ref-spec) is a special case - final String remoteName = (spec.getDestination() == null ? spec - .getSource() : spec.getDestination()); - final boolean forceUpdate = spec.isForceUpdate(); - final String localName = findTrackingRefName(remoteName, - fetchSpecs); - - final RemoteRefUpdate rru = new RemoteRefUpdate(db, srcRef, - remoteName, forceUpdate, localName, null); - result.add(rru); - } catch (TransportException x) { - throw x; - } catch (Exception x) { - throw new TransportException( - "Problem with resolving push ref spec \"" + spec - + "\" locally: " + x.getMessage(), x); - } + final String srcRef = spec.getSource(); + // null destination (no-colon in ref-spec) is a special case + final String remoteName = (spec.getDestination() == null ? spec + .getSource() : spec.getDestination()); + final boolean forceUpdate = spec.isForceUpdate(); + final String localName = findTrackingRefName(remoteName, fetchSpecs); + + final RemoteRefUpdate rru = new RemoteRefUpdate(db, srcRef, + remoteName, forceUpdate, localName, null); + result.add(rru); } return result; } @@ -638,7 +630,13 @@ public abstract class Transport { TransportException { if (toPush == null || toPush.isEmpty()) { // If the caller did not ask for anything use the defaults. - toPush = findRemoteRefUpdatesFor(push); + try { + toPush = findRemoteRefUpdatesFor(push); + } catch (final IOException e) { + throw new TransportException( + "Problem with resolving push ref specs locally: " + + e.getMessage(), e); + } if (toPush.isEmpty()) throw new TransportException("Nothing to push."); } @@ -660,12 +658,12 @@ public abstract class Transport { * @param specs * collection of RefSpec to convert. * @return collection of set up {@link RemoteRefUpdate}. - * @throws TransportException + * @throws IOException * when problem occurred during conversion or specification set * up: most probably, missing objects or refs. */ public Collection findRemoteRefUpdatesFor( - final Collection specs) throws TransportException { + final Collection specs) throws IOException { return findRemoteRefUpdatesFor(local, specs, fetch); } -- 2.11.4.GIT