From c3eb1cac794e2cf8efcb26695e8eb27ed80ece5c Mon Sep 17 00:00:00 2001 From: christ66 Date: Wed, 7 Jan 2015 15:17:58 -0800 Subject: [PATCH] If a project action fails to load we should log the project action and continue to load the project. --- .../java/hudson/model/AbstractProject.java | 9 +- .../main/java/hudson/model/Actionable.java | 11 ++- core/src/main/java/hudson/model/Project.java | 41 +++++++-- .../java/hudson/model/ItemGroupMixInTest.java | 83 ++++++++++++++++-- .../xmlFileReadExceptionOnLoad.zip | Bin 1545 -> 2854 bytes 5 files changed, 128 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java index 4da0cc2ef6..abea5887b7 100644 --- a/core/src/main/java/hudson/model/AbstractProject.java +++ b/core/src/main/java/hudson/model/AbstractProject.java @@ -751,8 +751,13 @@ public abstract class AbstractProject

,R extends A for (JobProperty p : Util.fixNull(properties)) ta.addAll(p.getJobActions((P)this)); - for (TransientProjectActionFactory tpaf : TransientProjectActionFactory.all()) - ta.addAll(Util.fixNull(tpaf.createFor(this))); // be defensive against null + for (TransientProjectActionFactory tpaf : TransientProjectActionFactory.all()) { + try { + ta.addAll(Util.fixNull(tpaf.createFor(this))); // be defensive against null + } catch (Exception e) { + LOGGER.log(Level.SEVERE, "Error loading transient project action factory."); + } + } return ta; } diff --git a/core/src/main/java/hudson/model/Actionable.java b/core/src/main/java/hudson/model/Actionable.java index ba19d39257..ca1332aa0f 100644 --- a/core/src/main/java/hudson/model/Actionable.java +++ b/core/src/main/java/hudson/model/Actionable.java @@ -30,6 +30,9 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.logging.Level; +import java.util.logging.Logger; + import jenkins.model.ModelObjectWithContextMenu; import jenkins.model.TransientActionFactory; import org.kohsuke.stapler.StaplerRequest; @@ -91,7 +94,11 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj List _actions = new ArrayList(getActions()); for (TransientActionFactory taf : ExtensionList.lookup(TransientActionFactory.class)) { if (taf.type().isInstance(this)) { - _actions.addAll(createFor(taf)); + try { + _actions.addAll(createFor(taf)); + } catch (Exception e) { + LOGGER.log(Level.SEVERE, "Error loading action.", e); + } } } return Collections.unmodifiableList(_actions); @@ -177,4 +184,6 @@ public abstract class Actionable extends AbstractModelObject implements ModelObj @Override public ContextMenu doContextMenu(StaplerRequest request, StaplerResponse response) throws Exception { return new ContextMenu().from(this,request,response); } + + private static final Logger LOGGER = Logger.getLogger(Actionable.class.getName()); } diff --git a/core/src/main/java/hudson/model/Project.java b/core/src/main/java/hudson/model/Project.java index 701ed893f7..894bb612d7 100644 --- a/core/src/main/java/hudson/model/Project.java +++ b/core/src/main/java/hudson/model/Project.java @@ -52,6 +52,9 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; +import java.util.logging.Level; +import java.util.logging.Logger; + import jenkins.triggers.SCMTriggerItem; /** @@ -234,15 +237,37 @@ public abstract class Project

,B extends Build> protected List createTransientActions() { List r = super.createTransientActions(); - for (BuildStep step : getBuildersList()) - r.addAll(step.getProjectActions(this)); - for (BuildStep step : getPublishersList()) - r.addAll(step.getProjectActions(this)); - for (BuildWrapper step : getBuildWrappers().values()) - r.addAll(step.getProjectActions(this)); - for (Trigger trigger : triggers()) - r.addAll(trigger.getProjectActions()); + for (BuildStep step : getBuildersList()) { + try { + r.addAll(step.getProjectActions(this)); + } catch (Exception e) { + LOGGER.log(Level.SEVERE, "Error loading build step.", e); + } + } + for (BuildStep step : getPublishersList()) { + try { + r.addAll(step.getProjectActions(this)); + } catch (Exception e) { + LOGGER.log(Level.SEVERE, "Error loading publisher.", e); + } + } + for (BuildWrapper step : getBuildWrappers().values()) { + try { + r.addAll(step.getProjectActions(this)); + } catch (Exception e) { + LOGGER.log(Level.SEVERE, "Error loading build wrapper.", e); + } + } + for (Trigger trigger : triggers()) { + try { + r.addAll(trigger.getProjectActions()); + } catch (Exception e) { + LOGGER.log(Level.SEVERE, "Error loading trigger.", e); + } + } return r; } + + private static final Logger LOGGER = Logger.getLogger(Project.class.getName()); } diff --git a/test/src/test/java/hudson/model/ItemGroupMixInTest.java b/test/src/test/java/hudson/model/ItemGroupMixInTest.java index b5514329d2..2a95dbe7a5 100644 --- a/test/src/test/java/hudson/model/ItemGroupMixInTest.java +++ b/test/src/test/java/hudson/model/ItemGroupMixInTest.java @@ -24,11 +24,15 @@ package hudson.model; -import java.util.Collection; -import static org.junit.Assert.*; import hudson.Extension; +import hudson.tasks.BuildStepDescriptor; +import hudson.tasks.BuildStepMonitor; +import hudson.tasks.BuildTrigger; import hudson.tasks.BuildWrapper; import hudson.tasks.BuildWrapperDescriptor; +import hudson.tasks.Builder; +import hudson.tasks.Publisher; +import hudson.triggers.Trigger; import org.apache.commons.io.FileUtils; import org.junit.Rule; import org.junit.Test; @@ -39,8 +43,14 @@ import org.jvnet.hudson.test.TestExtension; import org.jvnet.hudson.test.recipes.LocalData; import java.io.File; +import java.util.Collection; +import java.util.Iterator; import java.util.List; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + public class ItemGroupMixInTest { @Rule public JenkinsRule r = new JenkinsRule(); @@ -103,12 +113,17 @@ public class ItemGroupMixInTest { MockFolder d = r.jenkins.getItemByFullName("d", MockFolder.class); assertNotNull(d); Collection items = d.getItems(); - assertEquals(1, items.size()); - assertEquals("valid", items.iterator().next().getName()); + assertEquals(5, items.size()); + Iterator iterator = items.iterator(); + assertEquals("badBuildStep", iterator.next().getName()); + assertEquals("badBuildTrigger", iterator.next().getName()); + assertEquals("badBuildWrapper", iterator.next().getName()); + assertEquals("badPublisher", iterator.next().getName()); + assertEquals("valid", iterator.next().getName()); } @TestExtension - public static class MockBuilderThrowsError extends BuildWrapper { + public static class MockBuildWrapperThrowsError extends BuildWrapper { @Override public Collection getProjectActions(AbstractProject project){ throw new NullPointerException(); @@ -127,4 +142,62 @@ public class ItemGroupMixInTest { } } } + + @TestExtension + public static class MockBuilderThrowsError extends Builder { + @Override + public Collection getProjectActions(AbstractProject project){ + throw new NullPointerException(); + } + @Extension public static final Descriptor DESCRIPTOR = new DescriptorImpl(); + + public static class DescriptorImpl extends BuildStepDescriptor { + @Override + public boolean isApplicable(Class jobType) { + return false; + } + + @Override + public String getDisplayName() { + return null; + } + } + } + + @TestExtension + public static class MockBuildTriggerThrowsError extends Trigger { + @Override + public Collection getProjectActions() { + throw new NullPointerException(); + } + + @Extension public static final Descriptor DESCRIPTOR = new BuildTrigger.DescriptorImpl(); + } + + @TestExtension + public static class MockPublisherThrowsError extends Publisher { + @Override + public Collection getProjectActions(AbstractProject project) { + throw new NullPointerException(); + } + + @Override + public BuildStepMonitor getRequiredMonitorService() { + return null; + } + + @Extension public static final Descriptor DESCRIPTOR = new DescriptorImpl(); + + public static class DescriptorImpl extends BuildStepDescriptor { + @Override + public boolean isApplicable(Class jobType) { + return false; + } + + @Override + public String getDisplayName() { + return null; + } + } + } } diff --git a/test/src/test/resources/hudson/model/ItemGroupMixInTest/xmlFileReadExceptionOnLoad.zip b/test/src/test/resources/hudson/model/ItemGroupMixInTest/xmlFileReadExceptionOnLoad.zip index 70b2d12f980864ecc812c3c6b8c589a91d57d6d9..21a4e02c26b6940478b20acf97d8e4b64bb0399c 100644 GIT binary patch literal 2854 zcmWIWW@Zs#W&i@I9CbGk4Fjw|Mpk}Ov3>wfCG0RIDPTohU~@{VyPTiOW-zM&B|%sa zp(r^&FD)}&uOc_cCch|MFRLsswL~wYG^IE{Pp>4kxJ1u4KRMeiKPM%%NZ*bNVJ}}X zs?&H7Mxwc}HXl``B$~>k#1yB}%$$_qlGFmY$2b^3+O*UzJ>_I%U{GLUU{J=V4dIK{ zz>P)C20V8^i^hm8bJ$&~@MN9u?b8nJ(_)Uz+ZbAqoHIXVR!yDZ4d*_^+4nDBo;)w~ z&APo_{}?=__WH_iZZ}~(+dE;`-DSsr<$j*>?o?>v#Xn5tPqjR|j2rGYv+BAnWznji zp``JN)xN8CF>8tP8FimM{yUOfZkL*K7+Fcre3KmT>h;p7^HS%&EKjWc(tBZgi^??( zfsJAxLvp4(eE!@~tMHY>`?Ic-ofGyhuUT-TBz(Jx^)2hLz^x`l8n?wJIGHW}*}!qj zchQSt?{1a_UgCitEiZavFfzb@p$-w~9DLyG>FE=m_RDp4z zMnETWG}!tdHW0bS!yg9?a-g8oXrR!(rJ5Recy!si+xW%@7 z@Zy|OZ|pi@UHTsJ?{3^LY-0FlR7e*d?bwuhnbXMXqtB;nnqPm0=(zKy|2n+E>9b%j*}v)lyygaYy^S8S~bgGKFj^zOZl6%N)h?*0D@G*M~k_|4l9? zwfULL`3|<@d%miE*z~sH%&o$u;w|?7nPI`nz>tfUopD7(cu`_O0ZK%GbZRml;e7^- zfPcWqz!ecNoyZYk9dy`0VC_jwqn8`?6@nZj)LJzae7xMIiAB5Yzd2P+M&7eRI@o3E zz3};>f``plY`K4@liR_@fo1(cZRRciCrxwslD)4YTZd^`(=RU1oohEudXZ?J?t8Gj z^<~`Vt?&I0uZx`WCEw9XFEv6W*P~C&#Jgv;*{ub7{Wl|O=N*(kbT!y%!n(j?{&RLd z{eAzdV7}?b?7NdM_y{ZfwbibeDtK^t+QVt4dv8utJ+|w4%9+KcCdN6pBvX4@SN@w8 zQFob@cbm0ZfxbkM%$f5S=Qmt>{&&NjX^WRj9-cZ+konu`m48=7eX;oxj|k(5MFdNm zfYPL#%;F4`00wE3S@hmi5*WC(*h?FPHbm$y4ch1j4BflGMFXe%_$DM51aoq&aF$dE zb$@cpJ^9s*Yjd4{|2$H7#-WXG?wjxL_Nw2Xd-K($S8v!SOffzEXWx#~#s-JHQu|KV zgzwjp`xI)L%=gxuBmV5Bp1BNnq+}#+YKsPZJ!;CRtzM(g=f?h`I)ZP8%WTfGozp%q zo1Ef%(fDZe@~_7)m&utn%`QKEd&=wt4NuoU3mlh49diD-V6BC1nb+xB=Cs`NZ8l8# z%%W0W%U18oym2nJ(`!}c1ou)KrXU7encaD7Sz>c+wye*v-X1!=cV*+B=Sj1LUT3V4 z-jZJBwnakukcezY*VKm+$)}@>BWKDng_&g4?-)>OdGhlR(rXqQIu0C_gJT zxddE0F*4aRU46I(TkuyZy-JBDkqRjUY{N6~bmRkAQoW3Of*#(>=a2cI$6eT1#HLAc8e zn75z-0;XW@LexD7r(vso5Ke0#U=nuMVJm_Wt~(EGEn#yVtXM_F90#~CM=z2QuH$6E zZxVLbVJo~4t}_NU|Il2AD3g$#hOIC|I4ujGG1%ROEe|8yb%YJoU9cRD$RybEG{Qk& Yu5T-Pv znIeK_N@iYJVooMB2-<)q;Rr(!V1RNkgv?ZPvt7OH$TOh#e=;&K$N_1vV^Z|N6wEY4 zc(evu2OTyLSbI{_=;cOzg&+qBwN_09A1}9QV$m-9Z%$Q{k@q}l<|i0h6<-_X!n1$D zrLwOIehh`|f_vE3^IZ7m9>sJ~|L3E%8#!ihz0y{BdhL|gqUC4jn6lm%T~>YmTHQXL z*=d27>RDdyn35Q{MbR?qf{x+3TN%w(w&}@V3)%K_r~7j{?sAi~FMKxd{kGTI_p+0& zZ&F`4Q_W$QjdsOU!Gp`w9!^u;8#P_^*skX(XBL~980Xv)Ozml1`ES~ef6KU~a<=s&jGp!iit@8klS{x!jFCx>8CRZ?09phB0t|m0 zK{Pb2u|jefnyCTa2%|9b7qU_IFrz?84QLpa?163=YF>jF2267dTN)b>h9Uc&u(6nV z3fYbC5ym2X4a#A7e2tmEkWDlNx))pi00#jmw_!2ao*8#u13C~06dJZPHsdfE5gy3F ziJ2;qUAdGA)s?U`3JgyyDGA+N)D(&wO&_tEi*O-06tLRB$_DZ$2M{g