UploadPack v0: Extract "agent" client capability at parse time

The request receives a list of capabilities and takes out the "agent" to
offer it on its own setter (getAgent).

Do this at parse time: when reading the line if the capability is
"agent" set it directly in the builder.

This makes the treatment of "agent" consistent in v0/v1 and v2.

Change-Id: Ie4f9f2cad8639adeeaef4921df49a30a8ce5b42f
Signed-off-by: Ivan Frade <ifrade@google.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/parser/FirstWantTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/parser/FirstWantTest.java
index 627079eac193d53f7be83e20e0307bfcf96525e1..b877c598eff37c2fb5b922e9c7b3aebfc90682e0 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/parser/FirstWantTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/parser/FirstWantTest.java
@@ -43,6 +43,7 @@
 package org.eclipse.jgit.internal.transport.parser;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -66,9 +67,9 @@ public class FirstWantTest {
                                r.getLine());
                Set<String> capabilities = r.getCapabilities();
                Set<String> expectedCapabilities = new HashSet<>(
-                               Arrays.asList("no-progress", "include-tag", "ofs-delta",
-                                               "agent=JGit/unknown"));
+                               Arrays.asList("no-progress", "include-tag", "ofs-delta"));
                assertEquals(expectedCapabilities, capabilities);
+               assertEquals("JGit/unknown", r.getAgent());
        }
 
        @Test
@@ -79,6 +80,7 @@ public class FirstWantTest {
                assertEquals("want b9d4d1eb2f93058814480eae9e1b67550f46ee38",
                                r.getLine());
                assertTrue(r.getCapabilities().isEmpty());
+               assertNull(r.getAgent());
        }
 
        private String makeFirstWantLine(String capability) {
@@ -110,7 +112,7 @@ public class FirstWantTest {
                List<String> validNames = Arrays.asList(
                                "c", "cap", "C", "CAP", "1", "1cap", "cap-64k_test",
                                "-", "-cap",
-                               "_", "_cap", "agent=pack.age/Version");
+                               "_", "_cap");
 
                for (String capability: validNames) {
                        FirstWant r = FirstWant.fromLine(makeFirstWantLine(capability));
@@ -118,4 +120,11 @@ public class FirstWantTest {
                        assertTrue(r.getCapabilities().contains(capability));
                }
        }
+
+       @Test
+       public void testFirstWantValidAgentName() throws PackProtocolException {
+               FirstWant r = FirstWant.fromLine(makeFirstWantLine("agent=pack.age/Version"));
+               assertEquals(r.getCapabilities().size(), 0);
+               assertEquals("pack.age/Version", r.getAgent());
+       }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/parser/FirstWant.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/parser/FirstWant.java
index 1ac9b18874a634b101d68e670e36461f3545db65..2dae021702b392f0182699b609d5f9986f381493 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/parser/FirstWant.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/parser/FirstWant.java
@@ -42,11 +42,13 @@
  */
 package org.eclipse.jgit.internal.transport.parser;
 
-import java.util.Arrays;
+import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_AGENT;
+
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
+import org.eclipse.jgit.annotations.Nullable;
 import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.internal.JGitText;
 
@@ -72,6 +74,11 @@ public class FirstWant {
 
        private final Set<String> capabilities;
 
+       @Nullable
+       private final String agent;
+
+       private static final String AGENT_PREFIX = OPTION_AGENT + '=';
+
        /**
         * Parse the first want line in the protocol v0/v1 pack negotiation.
         *
@@ -84,6 +91,7 @@ public class FirstWant {
        public static FirstWant fromLine(String line) throws PackProtocolException {
                String wantLine;
                Set<String> capabilities;
+               String agent = null;
 
                if (line.length() > 45) {
                        String opt = line.substring(45);
@@ -91,8 +99,15 @@ public class FirstWant {
                                throw new PackProtocolException(JGitText.get().wantNoSpaceWithCapabilities);
                        }
                        opt = opt.substring(1);
-                       HashSet<String> opts = new HashSet<>(
-                                       Arrays.asList(opt.split(" "))); //$NON-NLS-1$
+
+                       HashSet<String> opts = new HashSet<>();
+                       for (String clientCapability : opt.split(" ")) { //$NON-NLS-1$
+                               if (clientCapability.startsWith(AGENT_PREFIX)) {
+                                       agent = clientCapability.substring(AGENT_PREFIX.length());
+                               } else {
+                                       opts.add(clientCapability);
+                               }
+                       }
                        wantLine = line.substring(0, 45);
                        capabilities = Collections.unmodifiableSet(opts);
                } else {
@@ -100,12 +115,14 @@ public class FirstWant {
                        capabilities = Collections.emptySet();
                }
 
-               return new FirstWant(wantLine, capabilities);
+               return new FirstWant(wantLine, capabilities, agent);
        }
 
-       private FirstWant(String line, Set<String> capabilities) {
+       private FirstWant(String line, Set<String> capabilities,
+                       @Nullable String agent) {
                this.line = line;
                this.capabilities = capabilities;
+               this.agent = agent;
        }
 
        /** @return non-capabilities part of the line. */
@@ -113,8 +130,17 @@ public class FirstWant {
                return line;
        }
 
-       /** @return capabilities parsed from the line as an immutable set. */
+       /**
+        * @return capabilities parsed from the line as an immutable set (excluding
+        *         agent).
+        */
        public Set<String> getCapabilities() {
                return capabilities;
        }
+
+       /** @return client user agent parsed from the line. */
+       @Nullable
+       public String getAgent() {
+               return agent;
+       }
 }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java
index 5a97036eb719db3039bd013f5ed29bca1a0304d4..05f4a8155fefa7f162dc79018d965b518a952144 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/FetchV0Request.java
@@ -114,14 +114,7 @@ final class FetchV0Request extends FetchRequest {
                 * @return this builder
                 */
                Builder addClientCapabilities(Collection<String> clientCapabilities) {
-                       for (String cap: clientCapabilities) {
-                               // TODO(ifrade): Do this is done on parse time
-                               if (cap.startsWith("agent=")) { //$NON-NLS-1$
-                                       agent = cap.substring("agent=".length()); //$NON-NLS-1$
-                               } else {
-                                       clientCaps.add(cap);
-                               }
-                       }
+                       clientCaps.addAll(clientCapabilities);
                        return this;
                }
 

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java
index 60d5fff6dd0815f63b043209658a37eff65a92f4..21498d6f5c4c1c91a0070c3ed4a51023cc260698 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/ProtocolV0Parser.java
@@ -143,6 +143,7 @@ final class ProtocolV0Parser {
                                if (line.length() > 45) {
                                        FirstWant firstLine = FirstWant.fromLine(line);
                                        reqBuilder.addClientCapabilities(firstLine.getCapabilities());
+                                       reqBuilder.setAgent(firstLine.getAgent());
                                        line = firstLine.getLine();
                                }
                        }

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index fde9ce91799c98afb315fa1a17f0c276479f3f1a..b69f2cd92fc3a37a2a981d5bfcde1e844c7c6a7d 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -209,6 +209,11 @@ public class UploadPack {
 
                /** @return capabilities parsed from the line. */
                public Set<String> getOptions() {
+                       if (firstWant.getAgent() != null) {
+                               Set<String> caps = new HashSet<>(firstWant.getCapabilities());
+                               caps.add(OPTION_AGENT + '=' + firstWant.getAgent());
+                               return caps;
+                       }
                        return firstWant.getCapabilities();
                }
        }