Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShawn Pearce2016-08-26 21:44:44 +0000
committerShawn Pearce2016-08-26 21:53:54 +0000
commitc2e2326a436d02aa94dcc566cbec96df1d0930cb (patch)
tree8ed1803d433760677544161653cb32423c7b6816
parent36cf4fe580a2b771331e0b9bcaea5505b3b7e27a (diff)
downloadjgit-c2e2326a436d02aa94dcc566cbec96df1d0930cb.tar.gz
jgit-c2e2326a436d02aa94dcc566cbec96df1d0930cb.tar.xz
jgit-c2e2326a436d02aa94dcc566cbec96df1d0930cb.zip
ReceivePack: refactor push option parsing
Refactor all of the push option support code to allocate the list immediately before parsing the options section off the stream. Move option support down to ReceivePack instead of BaseReceivePack. Push options are specific to the ReceivePack protocol and are not likely to appear in the 4 year old subscription proposal. These changes are OK before JGit 4.5 ships as no consumer should be relying on these new APIs. Change-Id: Ib07d18c877628aba07da07cd91875f918d509c49
-rw-r--r--org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java18
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java81
-rw-r--r--org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java52
3 files changed, 66 insertions, 85 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java
index 8ff0226183..c346d7904c 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushOptionsTest.java
@@ -84,7 +84,7 @@ public class PushOptionsTest extends RepositoryTestCase {
private InMemoryRepository client;
private ObjectId obj1;
private ObjectId obj2;
- private BaseReceivePack baseReceivePack;
+ private ReceivePack receivePack;
@Before
public void setUp() throws Exception {
@@ -96,13 +96,12 @@ public class PushOptionsTest extends RepositoryTestCase {
testProtocol = new TestProtocol<>(null,
new ReceivePackFactory<Object>() {
@Override
- public ReceivePack create(Object req, Repository database)
+ public ReceivePack create(Object req, Repository git)
throws ServiceNotEnabledException,
ServiceNotAuthorizedException {
- ReceivePack receivePack = new ReceivePack(database);
+ receivePack = new ReceivePack(git);
receivePack.setAllowPushOptions(true);
receivePack.setAtomic(true);
- baseReceivePack = receivePack;
return receivePack;
}
});
@@ -118,7 +117,6 @@ public class PushOptionsTest extends RepositoryTestCase {
@After
public void tearDown() {
- baseReceivePack = null;
Transport.unregister(testProtocol);
}
@@ -176,7 +174,7 @@ public class PushOptionsTest extends RepositoryTestCase {
assertSame(RemoteRefUpdate.Status.OK, one.getStatus());
assertSame(RemoteRefUpdate.Status.REJECTED_REMOTE_CHANGED,
two.getStatus());
- assertEquals(pushOptions, baseReceivePack.getPushOptions());
+ assertEquals(pushOptions, receivePack.getPushOptions());
}
@Test
@@ -197,7 +195,7 @@ public class PushOptionsTest extends RepositoryTestCase {
assertSame(RemoteRefUpdate.Status.OK, one.getStatus());
assertSame(RemoteRefUpdate.Status.OK, two.getStatus());
- assertEquals(pushOptions, baseReceivePack.getPushOptions());
+ assertEquals(pushOptions, receivePack.getPushOptions());
}
@Test
@@ -220,7 +218,7 @@ public class PushOptionsTest extends RepositoryTestCase {
one.getStatus());
assertSame(RemoteRefUpdate.Status.REJECTED_REMOTE_CHANGED,
two.getStatus());
- assertNull(baseReceivePack.getPushOptions());
+ assertNull(receivePack.getPushOptions());
}
@Test
@@ -241,7 +239,7 @@ public class PushOptionsTest extends RepositoryTestCase {
assertSame(RemoteRefUpdate.Status.OK, one.getStatus());
assertSame(RemoteRefUpdate.Status.REJECTED_REMOTE_CHANGED,
two.getStatus());
- assertEquals(pushOptions, baseReceivePack.getPushOptions());
+ assertEquals(pushOptions, receivePack.getPushOptions());
}
@Test
@@ -360,4 +358,4 @@ public class PushOptionsTest extends RepositoryTestCase {
fail("should already have thrown TransportException");
}
}
-} \ No newline at end of file
+}
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 825e294d98..5a61edd6ab 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
@@ -46,9 +46,9 @@ package org.eclipse.jgit.transport;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_ATOMIC;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_DELETE_REFS;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_OFS_DELTA;
+import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_PUSH_OPTIONS;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_QUIET;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REPORT_STATUS;
-import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_PUSH_OPTIONS;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SIDE_BAND_64K;
import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_AGENT;
import static org.eclipse.jgit.transport.SideBandOutputStream.CH_DATA;
@@ -69,7 +69,6 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
-import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.errors.InvalidObjectIdException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.PackProtocolException;
@@ -252,18 +251,6 @@ public abstract class BaseReceivePack {
private boolean quiet;
- /**
- * A list of option strings associated with a push.
- * @since 4.5
- */
- protected List<String> pushOptions;
-
- /**
- * Whether the client intends to use push options.
- * @since 4.5
- */
- protected boolean usePushOptions;
-
/** Lock around the received pack file, while updating refs. */
private PackLock packLock;
@@ -782,8 +769,7 @@ public abstract class BaseReceivePack {
* read.
*/
public boolean isSideBand() throws RequestNotYetReadException {
- if (enabledCapabilities == null)
- throw new RequestNotYetReadException();
+ checkRequestWasRead();
return enabledCapabilities.contains(CAPABILITY_SIDE_BAND_64K);
}
@@ -810,7 +796,7 @@ public abstract class BaseReceivePack {
}
/**
- * @return true if the server supports the receiving of push options.
+ * @return true if the server supports receiving push options.
* @since 4.5
*/
public boolean isAllowPushOptions() {
@@ -818,10 +804,10 @@ public abstract class BaseReceivePack {
}
/**
- * Configure if the server supports the receiving of push options.
+ * Configure if the server supports receiving push options.
*
* @param allow
- * true to permit option strings.
+ * true to optionally accept option strings from the client.
* @since 4.5
*/
public void setAllowPushOptions(boolean allow) {
@@ -840,45 +826,11 @@ public abstract class BaseReceivePack {
* @since 4.0
*/
public boolean isQuiet() throws RequestNotYetReadException {
- if (enabledCapabilities == null)
- throw new RequestNotYetReadException();
+ checkRequestWasRead();
return quiet;
}
/**
- * Gets an unmodifiable view of the option strings associated with the push.
- *
- * @return an unmodifiable view of pushOptions, or null (if pushOptions is).
- * @throws IllegalStateException
- * if allowPushOptions has not been set to true.
- * @throws RequestNotYetReadException
- * if the client's request has not yet been read from the wire,
- * so we do not know if they expect push options. Note that the
- * client may have already written the request, it just has not
- * been read.
- * @since 4.5
- */
- @Nullable
- public List<String> getPushOptions() {
- if (!allowPushOptions) {
- // Reading push options without a prior setAllowPushOptions(true)
- // call doesn't make sense.
- throw new IllegalStateException();
- }
- if (enabledCapabilities == null) {
- // Push options are not available until receive() has been called.
- throw new RequestNotYetReadException();
- }
- if (pushOptions == null) {
- // The client doesn't support push options. Return null to
- // distinguish this from the case where the client declared support
- // for push options and sent an empty list of them.
- return null;
- }
- return Collections.unmodifiableList(pushOptions);
- }
-
- /**
* Set the configuration for push certificate verification.
*
* @param cfg
@@ -1269,11 +1221,6 @@ public abstract class BaseReceivePack {
protected void enableCapabilities() {
sideBand = isCapabilityEnabled(CAPABILITY_SIDE_BAND_64K);
quiet = allowQuiet && isCapabilityEnabled(CAPABILITY_QUIET);
- usePushOptions = allowPushOptions
- && isCapabilityEnabled(CAPABILITY_PUSH_OPTIONS);
- if (usePushOptions) {
- pushOptions = new ArrayList<>();
- }
if (sideBand) {
OutputStream out = rawOut;
@@ -1287,17 +1234,6 @@ public abstract class BaseReceivePack {
}
/**
- * Sets the client's intention regarding push options.
- *
- * @param usePushOptions
- * whether the client intends to use push options
- * @since 4.5
- */
- public void setUsePushOptions(boolean usePushOptions) {
- this.usePushOptions = usePushOptions;
- }
-
- /**
* Check if the peer requested a capability.
*
* @param name
@@ -1308,6 +1244,11 @@ public abstract class BaseReceivePack {
return enabledCapabilities.contains(name);
}
+ void checkRequestWasRead() {
+ if (enabledCapabilities == null)
+ throw new RequestNotYetReadException();
+ }
+
/** @return true if a pack is expected based on the list of commands. */
protected boolean needPack() {
for (final ReceiveCommand cmd : commands) {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
index d16b723ff8..842c2d0626 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java
@@ -44,12 +44,17 @@
package org.eclipse.jgit.transport;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_ATOMIC;
+import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_PUSH_OPTIONS;
import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REPORT_STATUS;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import org.eclipse.jgit.annotations.Nullable;
import org.eclipse.jgit.errors.UnpackException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Repository;
@@ -71,6 +76,10 @@ public class ReceivePack extends BaseReceivePack {
private boolean echoCommandFailures;
+ /** Whether the client intends to use push options. */
+ private boolean usePushOptions;
+ private List<String> pushOptions;
+
/**
* Create a new pack receive for an open repository.
*
@@ -83,6 +92,36 @@ public class ReceivePack extends BaseReceivePack {
postReceive = PostReceiveHook.NULL;
}
+ /**
+ * Gets an unmodifiable view of the option strings associated with the push.
+ *
+ * @return an unmodifiable view of pushOptions, or null (if pushOptions is).
+ * @throws IllegalStateException
+ * if allowPushOptions has not been set to true.
+ * @throws RequestNotYetReadException
+ * if the client's request has not yet been read from the wire,
+ * so we do not know if they expect push options. Note that the
+ * client may have already written the request, it just has not
+ * been read.
+ * @since 4.5
+ */
+ @Nullable
+ public List<String> getPushOptions() {
+ if (!isAllowPushOptions()) {
+ // Reading push options without a prior setAllowPushOptions(true)
+ // call doesn't make sense.
+ throw new IllegalStateException();
+ }
+ checkRequestWasRead();
+ if (!usePushOptions) {
+ // The client doesn't support push options. Return null to
+ // distinguish this from the case where the client declared support
+ // for push options and sent an empty list of them.
+ return null;
+ }
+ return Collections.unmodifiableList(pushOptions);
+ }
+
/** @return the hook invoked before updates occur. */
public PreReceiveHook getPreReceiveHook() {
return preReceive;
@@ -171,15 +210,18 @@ public class ReceivePack extends BaseReceivePack {
@Override
protected void enableCapabilities() {
reportStatus = isCapabilityEnabled(CAPABILITY_REPORT_STATUS);
+ usePushOptions = isCapabilityEnabled(CAPABILITY_PUSH_OPTIONS);
super.enableCapabilities();
}
private void readPushOptions() throws IOException {
- String pushOption = pckIn.readString();
-
- while (pushOption != PacketLineIn.END) {
- pushOptions.add(pushOption);
- pushOption = pckIn.readString();
+ pushOptions = new ArrayList<>(4);
+ for (;;) {
+ String option = pckIn.readString();
+ if (option == PacketLineIn.END) {
+ break;
+ }
+ pushOptions.add(option);
}
}

Back to the top