[JENKINS-41124] Address code review comments

jenkinsci · Jan 21, 2017 · 32c5aba · 32c5aba
1 parent ba727fa
commit 32c5aba
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 12 deletions.
diff --git a/src/main/java/jenkins/branch/MultiBranchProjectFactory.java b/src/main/java/jenkins/branch/MultiBranchProjectFactory.java
@@ -203,7 +203,7 @@ public static abstract class BySCMSourceCriteria extends MultiBranchProjectFacto
          * Defines how to decide whether or not a given repository should host our type of project.
          *
          * @param source a repository
-         * @return criteria for treating its branches as a match
+         * @return criteria for treating its branches as a match or ({@code null} if all branches match)
          */
         @CheckForNull
         protected abstract SCMSourceCriteria getSCMSourceCriteria(@NonNull SCMSource source);

diff --git a/src/main/java/jenkins/branch/NameEncoder.java b/src/main/java/jenkins/branch/NameEncoder.java
@@ -24,9 +24,9 @@
 package jenkins.branch;
 
 /**
- * Encodes names that are not nice so that they are safe to use as url path segments.
+ * Encodes names that are not nice so that they are safe to use as URL path segments.
  * We don't want to do a full url encoding, only replace problematic names with {@code %} escaped variants so
- * that when double encoded by Stapler etc we bypass issues.
+ * that when double encoded by Stapler etc. we bypass issues.
  *
  * @since 2.0.0
  */
@@ -44,7 +44,7 @@ public static String encode(String name) {
             return "%00";
         }
         if (".".equals(name)) {
-            // just enough escaping to bypass the URL segment meaning of ".."
+            // just enough escaping to bypass the URL segment meaning of "."
             return "%2E";
         }
         if ("..".equals(name)) {
@@ -104,7 +104,7 @@ public static String encode(String name) {
                     buf.append("%2F");
                     break;
                 case '?':
-                buf.append("%3F");
+                    buf.append("%3F");
                 break;
                 case '[':
                     buf.append("%5B");
@@ -124,6 +124,9 @@ public static String encode(String name) {
     }
 
     public static String decode(String name) {
+        if (name.indexOf('%') == -1) {
+            return name;
+        }
         if ("%00".equals(name)) {
             return "";
         }
@@ -133,9 +136,6 @@ public static String decode(String name) {
         if ("%2E.".equals(name)) {
             return "..";
         }
-        if (name.indexOf('%') == -1) {
-            return name;
-        }
         StringBuilder buf = new StringBuilder(name.length() + 16);
         int matchIndex = 0;
         char match = 0;

diff --git a/src/test/java/integration/BrandingTest.java b/src/test/java/integration/BrandingTest.java
@@ -31,7 +31,6 @@
 import hudson.model.ListView;
 import hudson.model.Result;
 import hudson.model.TopLevelItem;
-import hudson.util.StreamTaskListener;
 import hudson.views.JobColumn;
 import hudson.views.StatusColumn;
 import hudson.views.WeatherColumn;

diff --git a/src/test/java/integration/MigrationTest.java b/src/test/java/integration/MigrationTest.java
@@ -232,7 +232,7 @@ public void nameMangling_folder_reload() throws Exception {
         r.addStep(new Statement() {
             @Override
             public void evaluate() throws Throwable {
-                OrganizationFolder foo = (OrganizationFolder) r.j.jenkins.getItem("foo");
+                OrganizationFolder foo = r.j.jenkins.getItemByFullName("foo", OrganizationFolder.class);
                 foo.doReload();
                 assertDataMigrated(foo);
             }
@@ -255,7 +255,7 @@ public void nameMangling_2_folder_reload() throws Exception {
         r.addStep(new Statement() {
             @Override
             public void evaluate() throws Throwable {
-                OrganizationFolder foo = (OrganizationFolder) r.j.jenkins.getItem("foo");
+                OrganizationFolder foo = r.j.jenkins.getItemByFullName("foo", OrganizationFolder.class);
                 foo.doReload();
                 assertDataMigrated(foo);
             }

diff --git a/src/test/java/jenkins/branch/WorkspaceLocatorImplTest.java b/src/test/java/jenkins/branch/WorkspaceLocatorImplTest.java
@@ -74,7 +74,7 @@ public void locate() throws Exception {
         stuff.scheduleBuild2(0).getFuture().get();
         r.waitUntilNoActivity();
         showComputation(stuff);
-        FreeStyleProject master = r.jenkins.getItemByFullName("stuff/dev%2fflow", FreeStyleProject.class);
+        FreeStyleProject master = r.jenkins.getItemByFullName("stuff/dev%2Fflow", FreeStyleProject.class);
         assertNotNull(master);
         assertEquals(r.jenkins.getRootPath().child("workspace/stuff_dev_flow-L5GKER67QGVMJ2UD3JCSGKEV2ACON2O4VO4RNUZ27HGUY32SYVXQ"), r.jenkins.getWorkspaceFor(master));
         DumbSlave slave = r.createOnlineSlave();