Merge pull request #72 from stephenc/jenkins-41124

[JENKINS-41124] Move name mangling support responsibility to AbstractFolder
jenkinsci · Jan 22, 2017 · 05030c1 · 05030c1
2 parents 228cd1a + 091997f
commit 05030c1
Show file tree

Hide file tree

Showing 16 changed files with 706 additions and 142 deletions.
diff --git a/pom.xml b/pom.xml
@@ -93,7 +93,7 @@
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>
       <artifactId>cloudbees-folder</artifactId>
-      <version>5.16</version>
+      <version>5.17-20170119.163248-6</version>
     </dependency>
     <dependency>
       <groupId>org.jenkins-ci.plugins</groupId>

diff --git a/src/main/java/jenkins/branch/Branch.java b/src/main/java/jenkins/branch/Branch.java
@@ -154,7 +154,7 @@ public String getName() {
      * @since 0.2-beta-7
      */
     public String getEncodedName() {
-        return NameMangler.apply(getName());
+        return NameEncoder.encode(getName());
     }
 
     /**

diff --git a/src/main/java/jenkins/branch/MultiBranchProject.java b/src/main/java/jenkins/branch/MultiBranchProject.java
@@ -24,6 +24,7 @@
 
 package jenkins.branch;
 
+import com.cloudbees.hudson.plugins.folder.ChildNameGenerator;
 import com.cloudbees.hudson.plugins.folder.FolderIcon;
 import com.cloudbees.hudson.plugins.folder.computed.ChildObserver;
 import com.cloudbees.hudson.plugins.folder.computed.ComputedFolder;
@@ -164,34 +165,13 @@ protected MultiBranchProject(ItemGroup parent, String name) {
     public void onLoad(ItemGroup<? extends Item> parent, String name) throws IOException {
         super.onLoad(parent, name);
         init2();
-        // migrate any non-mangled names
-        ArrayList<P> items = new ArrayList<>(getItems());
-        BranchProjectFactory<P, R> projectFactory = getProjectFactory();
-        for (P item : items) {
-            if (projectFactory.isProject(item)) {
-                String itemName = item.getName();
-                Branch branch = projectFactory.getBranch(item);
-                String mangledName = branch.getEncodedName();
-                if (!itemName.equals(mangledName)) {
-                    if (super.getItem(mangledName) == null) {
-                        LOGGER.log(Level.INFO, "Non-mangled name detected for branch {0}. Renaming {1}/{2} to {1}/{3}",
-                                new Object[]{branch.getName(), getFullName(), itemName, mangledName});
-                        item.renameTo(mangledName);
-                        if (item.getDisplayNameOrNull() == null) {
-                            item.setDisplayName(branch.getName());
-                            item.save();
-                        }
-                    } // else will be removed by the orphaned item strategy on next scan
-                }
-            }
-        }
         try {
             srcDigest = Util.getDigestOf(Items.XSTREAM2.toXML(sources));
         } catch (XStreamException e) {
             srcDigest = null;
         }
         try {
-            facDigest = Util.getDigestOf(Items.XSTREAM2.toXML(projectFactory));
+            facDigest = Util.getDigestOf(Items.XSTREAM2.toXML(getProjectFactory()));
         } catch (XStreamException e) {
             facDigest = null;
         }
@@ -707,23 +687,19 @@ public P getItem(String name) {
         if (name == null) {
             return null;
         }
+        P item = super.getItem(name);
+        if (item != null) {
+            return item;
+        }
         if (name.indexOf('%') != -1) {
-            String decoded = rawDecode(name);
-            P item = super.getItem(decoded);
-            if (item != null) {
-                return item;
-            }
-            item = super.getItem(NameMangler.apply(decoded));
+            String decoded = NameEncoder.decode(name);
+            item = super.getItem(decoded);
             if (item != null) {
                 return item;
             }
-            // fall through for double decoded call paths
+            // fall through for double decoded call paths // TODO is this necessary
         }
-        P item = super.getItem(name);
-        if (item != null) {
-            return item;
-        }
-        return super.getItem(NameMangler.apply(name));
+        return super.getItem(NameEncoder.encode(name));
     }
 
     /**
@@ -736,17 +712,7 @@ public P getItem(String name) {
      */
     @CheckForNull
     public P getItemByBranchName(@NonNull String branchName) {
-        BranchProjectFactory<P, R> factory = getProjectFactory();
-        P item = getItem(NameMangler.apply(branchName));
-        if (item != null && factory.isProject(item) && branchName.equals(factory.getBranch(item).getName())) {
-            return item;
-        }
-        for (P p: getItems()) {
-            if (factory.isProject(p) && branchName.equals(factory.getBranch(p).getName())) {
-                return p;
-            }
-        }
-        return null;
+        return super.getItem(NameEncoder.encode(branchName));
     }
 
     /**
@@ -1962,7 +1928,11 @@ public void observe(@NonNull SCMHead head, @NonNull SCMRevision revision) {
                 listener.getLogger().println("Ignoring duplicate branch project " + rawName);
                 return;
             }
-            project = _factory.newInstance(branch);
+            try (ChildNameGenerator.Trace trace = ChildNameGenerator.beforeCreateItem(
+                    MultiBranchProject.this, branch.getEncodedName(), branch.getName()
+            )){
+                project = _factory.newInstance(branch);
+            }
             if (!project.getName().equals(encodedName)) {
                 throw new IllegalStateException(
                         "Name of created project " + project + " did not match expected " + encodedName);

diff --git a/src/main/java/jenkins/branch/MultiBranchProjectDescriptor.java b/src/main/java/jenkins/branch/MultiBranchProjectDescriptor.java
@@ -23,18 +23,26 @@
  */
 package jenkins.branch;
 
+import com.cloudbees.hudson.plugins.folder.AbstractFolder;
 import com.cloudbees.hudson.plugins.folder.AbstractFolderDescriptor;
+import com.cloudbees.hudson.plugins.folder.ChildNameGenerator;
 import com.cloudbees.hudson.plugins.folder.FolderIconDescriptor;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.ExtensionList;
+import hudson.Util;
 import hudson.model.Descriptor;
 import hudson.model.Job;
+import hudson.model.Run;
+import hudson.model.TopLevelItem;
 import hudson.model.TopLevelItemDescriptor;
+import java.io.IOException;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.WeakHashMap;
+import javax.annotation.CheckForNull;
 import jenkins.model.Jenkins;
 import jenkins.scm.api.SCMSourceDescriptor;
 import org.jvnet.tiger_types.Types;
@@ -206,4 +214,64 @@ public List<FolderIconDescriptor> getIconDescriptors() {
     public boolean isIconConfigurable() {
         return false;
     }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    @NonNull
+    public <I extends TopLevelItem> ChildNameGenerator<AbstractFolder<I>,I> childNameGenerator() {
+        return (ChildNameGenerator<AbstractFolder<I>, I>)ChildNameGeneratorImpl.INSTANCE;
+    }
+
+    public static class ChildNameGeneratorImpl<P extends Job<P, R> & TopLevelItem,
+            R extends Run<P, R>> extends ChildNameGenerator<MultiBranchProject<P,R>,P> {
+
+        /*package*/ static final ChildNameGeneratorImpl INSTANCE = new ChildNameGeneratorImpl();
+
+        @Override
+        @CheckForNull
+        public String itemNameFromItem(@NonNull MultiBranchProject<P,R> parent, @NonNull P item) {
+            BranchProjectFactory<P, R> factory = parent.getProjectFactory();
+            if (factory.isProject(item)) {
+                return NameEncoder.encode(factory.getBranch(item).getName());
+            }
+            String idealName = idealNameFromItem(parent, item);
+            if (idealName != null) {
+                return NameEncoder.encode(idealName);
+            }
+            return null;
+        }
+
+        @Override
+        @CheckForNull
+        public String dirNameFromItem(@NonNull MultiBranchProject<P,R> parent, @NonNull P item) {
+            BranchProjectFactory<P, R> factory = parent.getProjectFactory();
+            if (factory.isProject(item)) {
+                return NameMangler.apply(factory.getBranch(item).getName());
+            }
+            String idealName = idealNameFromItem(parent, item);
+            if (idealName != null) {
+                return NameMangler.apply(idealName);
+            }
+            return null;
+        }
+
+        @Override
+        @NonNull
+        public String itemNameFromLegacy(@NonNull MultiBranchProject<P, R> parent, @NonNull String legacyDirName) {
+            return NameEncoder.decode(legacyDirName);
+        }
+
+        @Override
+        @NonNull
+        public String dirNameFromLegacy(@NonNull MultiBranchProject<P, R> parent, @NonNull String legacyDirName) {
+            return NameMangler.apply(NameEncoder.decode(legacyDirName));
+        }
+
+        @Override
+        public void recordLegacyName(MultiBranchProject<P, R> parent, P item, String legacyDirName) throws IOException {
+            // no-op because we already tracked the name in Branch.getName()
+        }
+
+    }
+
 }
diff --git a/src/main/java/jenkins/branch/MultiBranchProjectFactory.java b/src/main/java/jenkins/branch/MultiBranchProjectFactory.java
@@ -203,9 +203,9 @@ 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)
          */
-        @NonNull
+        @CheckForNull
         protected abstract SCMSourceCriteria getSCMSourceCriteria(@NonNull SCMSource source);
 
         /**