Merge pull request #45 from jglick/MultiBranchProjectFactory-update-J…

…ENKINS-34246 [JENKINS-34246] Fix MultiBranchProjectFactory API to allow deletion or update of existing children
jenkinsci · Jun 9, 2016 · 2d3afb7 · 2d3afb7
2 parents 0c390a0 + 91a0459
commit 2d3afb7
Show file tree

Hide file tree

Showing 3 changed files with 126 additions and 48 deletions.
diff --git a/src/main/java/jenkins/branch/MultiBranchProjectFactory.java b/src/main/java/jenkins/branch/MultiBranchProjectFactory.java
@@ -25,6 +25,7 @@
 package jenkins.branch;
 
 import hudson.ExtensionPoint;
+import hudson.Util;
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.ItemGroup;
 import hudson.model.TaskListener;
@@ -46,21 +47,67 @@
  * how to get started using the type of project factory. This view is displayed when there are no subfolders found.
  *
  * @since 0.2-beta-5
+ * @see OrganizationFolder#getProjectFactories
  */
 public abstract class MultiBranchProjectFactory extends AbstractDescribableImpl<MultiBranchProjectFactory> implements ExtensionPoint {
 
     /**
-     * Creates a multibranch project for a given set of SCM sources if they seem compatible.
+     * Determines whether this factory recognizes a given configuration.
      * @param parent a folder
      * @param name a project name
      * @param scmSources a set of SCM sources as added by {@link jenkins.scm.api.SCMSourceObserver.ProjectObserver#addSource}
      * @param attributes a set of metadata attributes as added by {@link jenkins.scm.api.SCMSourceObserver.ProjectObserver#addAttribute}
      * @param listener a way of reporting progress
-     * @return a new uninitialized multibranch project (do not configure its {@link MultiBranchProject#getSourcesList} or call {@link MultiBranchProject#onCreatedFromScratch}), or null if unrecognized
+     * @return true if recognized
      */
+    public boolean recognizes(@Nonnull ItemGroup<?> parent, @Nonnull String name, @Nonnull List<? extends SCMSource> scmSources, @Nonnull Map<String,Object> attributes, @Nonnull TaskListener listener) throws IOException, InterruptedException {
+        if (Util.isOverridden(MultiBranchProjectFactory.class, getClass(), "createProject", ItemGroup.class, String.class, List.class, Map.class, TaskListener.class)) {
+            return createProject(parent, name, scmSources, attributes, listener) != null;
+        } else {
+            throw new AbstractMethodError(getClass().getName() + " must override recognizes");
+        }
+    }
+
+    /**
+     * Creates a new multibranch project which matches {@link #recognizes}.
+     * @param parent a folder
+     * @param name a project name
+     * @param scmSources a set of SCM sources as added by {@link jenkins.scm.api.SCMSourceObserver.ProjectObserver#addSource}
+     * @param attributes a set of metadata attributes as added by {@link jenkins.scm.api.SCMSourceObserver.ProjectObserver#addAttribute}
+     * @param listener a way of reporting progress
+     * @return a new uninitialized multibranch project (do not configure its {@link MultiBranchProject#getSourcesList} or call {@link MultiBranchProject#onCreatedFromScratch})
+     */
+    @Nonnull
+    public MultiBranchProject<?,?> createNewProject(@Nonnull ItemGroup<?> parent, @Nonnull String name, @Nonnull List<? extends SCMSource> scmSources, @Nonnull Map<String,Object> attributes, @Nonnull TaskListener listener) throws IOException, InterruptedException {
+        if (Util.isOverridden(MultiBranchProjectFactory.class, getClass(), "createProject", ItemGroup.class, String.class, List.class, Map.class, TaskListener.class)) {
+            MultiBranchProject<?, ?> p = createProject(parent, name, scmSources, attributes, listener);
+            if (p == null) {
+                throw new IOException("recognized project " + name + " before, but now");
+            }
+            return p;
+        } else {
+            throw new AbstractMethodError(getClass().getName() + " must override createNewProject");
+        }
+    }
+
+    /**
+     * Updates an existing project which matches {@link #recognizes}.
+     * @param project an existing project, perhaps created by this factory, perhaps not
+     * @param attributes a set of metadata attributes as added by {@link jenkins.scm.api.SCMSourceObserver.ProjectObserver#addAttribute}
+     * @param listener a way of reporting progress
+     */
+    public void updateExistingProject(MultiBranchProject<?,?> project, @Nonnull Map<String,Object> attributes, @Nonnull TaskListener listener) throws IOException, InterruptedException {}
+
+    @Deprecated
     @CheckForNull
-    public abstract MultiBranchProject<?,?> createProject(@Nonnull ItemGroup<?> parent, @Nonnull String name, @Nonnull List<? extends SCMSource> scmSources, @Nonnull Map<String,Object> attributes, @Nonnull TaskListener listener) throws IOException, InterruptedException;
-
+    public MultiBranchProject<?,?> createProject(@Nonnull ItemGroup<?> parent, @Nonnull String name, @Nonnull List<? extends SCMSource> scmSources, @Nonnull Map<String,Object> attributes, @Nonnull TaskListener listener) throws IOException, InterruptedException {
+        if (recognizes(parent, name, scmSources, attributes, listener)) {
+            return createNewProject(parent, name, scmSources, attributes, listener);
+        } else {
+            return null;
+        }
+    }
+
     @Override
     public MultiBranchProjectFactoryDescriptor getDescriptor() {
         return (MultiBranchProjectFactoryDescriptor) super.getDescriptor();
@@ -85,17 +132,18 @@ public static abstract class BySCMSourceCriteria extends MultiBranchProjectFacto
         protected abstract SCMSourceCriteria getSCMSourceCriteria(@Nonnull SCMSource source);
 
         /**
-         * Creates a project given that there seems to be a match.
-         * @param parent the folder
-         * @param name the project name
-         * @param attributes a set of metadata attributes as added by {@link jenkins.scm.api.SCMSourceObserver.ProjectObserver#addAttribute}
-         * @return a new project suitable for {@link #createProject}
+         * Historical alias for {@link #createNewProject}.
          */
         @Nonnull
         protected abstract MultiBranchProject<?,?> doCreateProject(@Nonnull ItemGroup<?> parent, @Nonnull String name, @Nonnull Map<String,Object> attributes);
 
         @Override
-        public final MultiBranchProject<?,?> createProject(ItemGroup<?> parent, String name, List<? extends SCMSource> scmSources, @Nonnull Map<String,Object> attributes, final TaskListener listener) throws IOException, InterruptedException {
+        public final MultiBranchProject<?, ?> createNewProject(ItemGroup<?> parent, String name, List<? extends SCMSource> scmSources, Map<String, Object> attributes, TaskListener listener) throws IOException, InterruptedException {
+            return doCreateProject(parent, name, attributes);
+        }
+
+        @Override
+        public boolean recognizes(ItemGroup<?> parent, String name, List<? extends SCMSource> scmSources, Map<String, Object> attributes, final TaskListener listener) throws IOException, InterruptedException {
             for (final SCMSource scmSource : scmSources) {
                 SCMSourceOwner owner = scmSource.getOwner();
                 if (!(owner instanceof SCMSourceOwnerHack)) {
@@ -120,10 +168,10 @@ public Boolean call() throws Exception {
                     throw new AssertionError(x);
                 }
                 if (!empty) {
-                    return doCreateProject(parent, name, attributes);
+                    return true;
                 }
             }
-            return null;
+            return false;
         }
 
     }

diff --git a/src/main/java/jenkins/branch/OrganizationFolder.java b/src/main/java/jenkins/branch/OrganizationFolder.java
@@ -158,41 +158,46 @@ public void addAttribute(String key, Object value) throws IllegalArgumentExcepti
                         }
                         @Override
                         public void complete() throws IllegalStateException, InterruptedException {
-                            MultiBranchProject<?,?> existing = observer.shouldUpdate(projectName);
-                            if (existing != null) {
-                                PersistedList<BranchSource> sourcesList = existing.getSourcesList();
-                                sourcesList.clear();
-                                sourcesList.addAll(createBranchSources());
-                                existing.setOrphanedItemStrategy(getOrphanedItemStrategy());
-                                existing.scheduleBuild();
-                                return;
-                            }
-                            if (!observer.mayCreate(projectName)) {
-                                listener.getLogger().println("Ignoring duplicate child " + projectName);
-                                return;
-                            }
-                            for (MultiBranchProjectFactory factory : projectFactories) {
-                                MultiBranchProject<?, ?> project;
-                                try {
-                                    project = factory.createProject(OrganizationFolder.this, projectName, sources, Collections.<String,Object>emptyMap(), listener);
-                                } catch (InterruptedException x) {
-                                    throw x;
-                                } catch (Exception x) {
-                                    x.printStackTrace(listener.error("Failed to create a subproject " + projectName));
-                                    continue;
-                                }
-                                if (project != null) {
-                                    project.setOrphanedItemStrategy(getOrphanedItemStrategy());
-                                    project.getSourcesList().addAll(createBranchSources());
-                                    try {
-                                        project.addTrigger(new PeriodicFolderTrigger("1d"));
-                                    } catch (ANTLRException x) {
-                                        throw new IllegalStateException(x);
+                            try {
+                                MultiBranchProjectFactory factory = null;
+                                Map<String, Object> attributes = Collections.<String,Object>emptyMap();
+                                for (MultiBranchProjectFactory candidateFactory : projectFactories) {
+                                    if (candidateFactory.recognizes(OrganizationFolder.this, projectName, sources, attributes, listener)) {
+                                        factory = candidateFactory;
+                                        break;
                                     }
-                                    observer.created(project);
-                                    project.scheduleBuild();
-                                    break;
                                 }
+                                if (factory == null) {
+                                    return;
+                                }
+                                MultiBranchProject<?,?> existing = observer.shouldUpdate(projectName);
+                                if (existing != null) {
+                                    PersistedList<BranchSource> sourcesList = existing.getSourcesList();
+                                    sourcesList.clear();
+                                    sourcesList.addAll(createBranchSources());
+                                    existing.setOrphanedItemStrategy(getOrphanedItemStrategy());
+                                    factory.updateExistingProject(existing, attributes, listener);
+                                    existing.scheduleBuild();
+                                    return;
+                                }
+                                if (!observer.mayCreate(projectName)) {
+                                    listener.getLogger().println("Ignoring duplicate child " + projectName);
+                                    return;
+                                }
+                                MultiBranchProject<?, ?> project = factory.createNewProject(OrganizationFolder.this, projectName, sources, attributes, listener);
+                                project.setOrphanedItemStrategy(getOrphanedItemStrategy());
+                                project.getSourcesList().addAll(createBranchSources());
+                                try {
+                                    project.addTrigger(new PeriodicFolderTrigger("1d"));
+                                } catch (ANTLRException x) {
+                                    throw new IllegalStateException(x);
+                                }
+                                observer.created(project);
+                                project.scheduleBuild();
+                            } catch (InterruptedException x) {
+                                throw x;
+                            } catch (Exception x) {
+                                x.printStackTrace(listener.error("Failed to create or update a subproject " + projectName));
                             }
                         }
                     };

diff --git a/src/test/java/jenkins/branch/OrganizationFolderTest.java b/src/test/java/jenkins/branch/OrganizationFolderTest.java
@@ -82,8 +82,6 @@ public void configRoundTrip() throws Exception {
     @Issue("JENKINS-31516")
     public void indexChildrenOnOrganizationFolderIndex() throws Exception {
         OrganizationFolder top = r.jenkins.createProject(OrganizationFolder.class, "top");
-        List<MultiBranchProjectFactory> projectFactories = top.getProjectFactories();
-        projectFactories.add(new MockFactory());
         top.getNavigators().add(new SingleSCMNavigator("stuff", Collections.<SCMSource>singletonList(new SingleSCMSource("id", "stuffy", new NullSCM()))));
         top = r.configRoundtrip(top);
 
@@ -104,14 +102,41 @@ public void emptyViewEquality() throws Exception {
         assertTrue(emptyView.isDefault());
     }
 
+    @Issue("JENKINS-34246")
+    @Test
+    public void deletedMarker() throws Exception {
+        OrganizationFolder top = r.jenkins.createProject(OrganizationFolder.class, "top");
+        List<MultiBranchProjectFactory> projectFactories = top.getProjectFactories();
+        assertEquals(1, projectFactories.size());
+        assertEquals(MockFactory.class, projectFactories.get(0).getClass());
+        top.getNavigators().add(new SingleSCMNavigator("stuff", Collections.<SCMSource>singletonList(new SingleSCMSource("id", "stuffy", new NullSCM()))));
+        top.scheduleBuild2(0).getFuture().get();
+        top.getComputation().writeWholeLogTo(System.out);
+        assertEquals(1, top.getItems().size());
+        r.waitUntilNoActivity();
+        MockFactory.live = false;
+        try {
+            top.scheduleBuild2(0).getFuture().get();
+            top.getComputation().writeWholeLogTo(System.out);
+            assertEquals(0, top.getItems().size());
+        } finally {
+            MockFactory.live = true;
+        }
+    }
+
     @TestExtension
     public static class ConfigRoundTripDescriptor extends MockFactoryDescriptor {}
 
     public static class MockFactory extends MultiBranchProjectFactory {
         @DataBoundConstructor
         public MockFactory() {}
+        static boolean live = true;
+        @Override
+        public boolean recognizes(ItemGroup<?> parent, String name, List<? extends SCMSource> scmSources, Map<String, Object> attributes, TaskListener listener) throws IOException, InterruptedException {
+            return live;
+        }
         @Override
-        public MultiBranchProject<?, ?> createProject(ItemGroup<?> parent, String name, List<? extends SCMSource> scmSources, Map<String,Object> attributes, TaskListener listener) throws IOException, InterruptedException {
+        public MultiBranchProject<?, ?> createNewProject(ItemGroup<?> parent, String name, List<? extends SCMSource> scmSources, Map<String,Object> attributes, TaskListener listener) throws IOException, InterruptedException {
             return new MultiBranchImpl(parent, name);
         }
     }