Merge pull request #73 from stephenc/jenkins-41209

[JENKINS-41209] Null Pointer Exception when deserializing during upgrade

src/main/java/jenkins/branch/Branch.java

@@ -82,7 +82,7 @@ public class Branch {
      * representation is simpler.
      * @since 2.0
      */
-    private List<Action> actions = new ArrayList<>();
+    private List<Action> actions;
 
     /**
      * Constructs a branch instance.
@@ -97,6 +97,7 @@ public Branch(String sourceId, SCMHead head, SCM scm, List<? extends BranchPrope
         this.head = head;
         this.scm = scm;
         this.properties.addAll(properties);
+        this.actions = new ArrayList<>();
     }
 
     /**
@@ -122,7 +123,7 @@ private Branch(String id, SCM scm, Branch b) {
      */
     private Object readResolve() throws ObjectStreamException {
         if (actions == null) {
-            return new Branch(sourceId, head, scm, properties);
+            actions = new ArrayList<>();
         }
         return this;
     }
@@ -220,7 +221,7 @@ public List<BranchProperty> getProperties() {
      */
     @NonNull
     public List<Action> getActions() {
-        return Collections.unmodifiableList(actions);
+        return actions == null ? Collections.<Action>emptyList() : Collections.unmodifiableList(actions);
     }
 
     /**
@@ -240,6 +241,9 @@ public List<Action> getActions() {
      */
     @CheckForNull
     public <T extends Action> T getAction(Class<T> clazz) {
+        if (actions == null) {
+            return null;
+        }
         for (Action p : actions) {
             if (clazz.isInstance(p)) {
                 return clazz.cast(p);
@@ -315,21 +319,6 @@ public Dead(Branch b) {
             super(NullSCMSource.ID, new NullSCM(), b);
         }
 
-        /**
-         * Ensure actions is never null.
-         *
-         * @return the deserialized object.
-         * @throws ObjectStreamException if things go wrong.
-         */
-        @SuppressWarnings("ConstantConditions")
-        @SuppressFBWarnings("RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE")
-        private Object readResolve() throws ObjectStreamException {
-            if (getActions() == null) {
-                return new Branch.Dead(getHead(), getProperties());
-            }
-            return this;
-        }
-
         /**
          * {@inheritDoc}
          */

src/test/java/jenkins/branch/BranchTest.java

@@ -0,0 +1,82 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package jenkins.branch;
+
+import hudson.model.Action;
+import hudson.model.Items;
+import hudson.scm.NullSCM;
+import java.util.Collections;
+import jenkins.scm.impl.NullSCMSource;
+import jenkins.scm.impl.mock.MockSCMHead;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+
+public class BranchTest {
+
+    @Test
+    public void given_livePre2Branch_when_deserialized_then_objectInvariantsCorrect() throws Exception {
+        Branch b = (Branch) Items.XSTREAM2
+                .fromXML("<branch class=\"jenkins.branch.Branch\" >\n"
+                        + "        <sourceId>any-id</sourceId>\n"
+                        + "        <head class=\"jenkins.scm.impl.mock.MockSCMHead\">\n"
+                        + "          <name>quicker</name>\n"
+                        + "        </head>\n"
+                        + "        <scm class=\"hudson.scm.NullSCM\"/>\n"
+                        + "        <properties class=\"java.util.concurrent.CopyOnWriteArrayList\">\n"
+                        + "          <jenkins.branch.NoTriggerBranchProperty/>\n"
+                        + "        </properties>\n"
+                        + "      </branch>");
+        assertThat(b.getName(), is("quicker"));
+        assertThat(b.getHead(), instanceOf(MockSCMHead.class));
+        assertThat(b.getActions(), is(Collections.<Action>emptyList()));
+        assertThat(b.getScm(), instanceOf(NullSCM.class));
+        assertThat(b.getSourceId(), is("any-id"));
+        assertThat(b.getProperties(), contains(instanceOf(NoTriggerBranchProperty.class)));
+    }
+
+    @Test
+    public void given_deadPre2Branch_when_deserialized_then_objectInvariantsCorrect() throws Exception {
+        Branch b = (Branch) Items.XSTREAM2
+                .fromXML("<branch class=\"jenkins.branch.Branch$Dead\" >\n"
+                        + "        <sourceId>::NullSCMSource::</sourceId>\n"
+                        + "        <head class=\"jenkins.scm.impl.mock.MockSCMHead\">\n"
+                        + "          <name>quicker</name>\n"
+                        + "        </head>\n"
+                        + "        <scm class=\"hudson.scm.NullSCM\"/>\n"
+                        + "        <properties class=\"java.util.concurrent.CopyOnWriteArrayList\">\n"
+                        + "          <jenkins.branch.NoTriggerBranchProperty/>\n"
+                        + "        </properties>\n"
+                        + "      </branch>");
+        assertThat(b.getName(), is("quicker"));
+        assertThat(b.getHead(), instanceOf(MockSCMHead.class));
+        assertThat(b.getActions(), is(Collections.<Action>emptyList()));
+        assertThat(b.getScm(), instanceOf(NullSCM.class));
+        assertThat(b.getSourceId(), is(NullSCMSource.ID));
+        assertThat(b.getProperties(), contains(instanceOf(NoTriggerBranchProperty.class)));
+    }
+}