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 8ff022618..c346d7904 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 void setUp() throws Exception { testProtocol = new TestProtocol<>(null, new ReceivePackFactory() { @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 ReceivePack create(Object req, Repository database) @After public void tearDown() { - baseReceivePack = null; Transport.unregister(testProtocol); } @@ -176,7 +174,7 @@ public void testNonAtomicPushWithOptions() throws Exception { 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 void testAtomicPushWithOptions() throws Exception { 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 void testFailedAtomicPushWithOptions() throws Exception { one.getStatus()); assertSame(RemoteRefUpdate.Status.REJECTED_REMOTE_CHANGED, two.getStatus()); - assertNull(baseReceivePack.getPushOptions()); + assertNull(receivePack.getPushOptions()); } @Test @@ -241,7 +239,7 @@ public void testThinPushWithOptions() throws Exception { 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 void testPushOptionsNotSupported() throws Exception { 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 825e294d9..5a61edd6a 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 @@ 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.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 Set getCapabilities() { private boolean quiet; - /** - * A list of option strings associated with a push. - * @since 4.5 - */ - protected List 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 void setMaxPackSizeLimit(final long limit) { * 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 void setAllowQuiet(boolean allow) { } /** - * @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 boolean isAllowPushOptions() { } /** - * 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,44 +826,10 @@ public void setAllowPushOptions(boolean allow) { * @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 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. * @@ -1269,11 +1221,6 @@ static ReceiveCommand parseCommand(String line) throws PackProtocolException { 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; @@ -1286,17 +1233,6 @@ protected void enableCapabilities() { } } - /** - * 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. * @@ -1308,6 +1244,11 @@ protected boolean isCapabilityEnabled(String name) { 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 d16b723ff..842c2d062 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 pushOptions; + /** * Create a new pack receive for an open repository. * @@ -83,6 +92,36 @@ public ReceivePack(final Repository into) { 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 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 void receive(final InputStream input, final OutputStream output, @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); } }