Merge pull request #8 from amuniz/JENKINS-30424

[JENKINS-30424] Move build.save to the migration thread

src/main/java/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2.java

@@ -101,6 +101,20 @@ static void migrate(PersistenceRoot pr) throws IDStoreMigrationException {
        }
     }
 
+    static void saveIfNeeded(Run run) throws IOException {
+        Jenkins jenkins = Jenkins.getInstance();
+        if (jenkins == null) {
+            throw new IllegalStateException("Jenkins is null, so it is impossible to migrate the ID");
+        }
+        File marker = new File(jenkins.getRootDir(), MARKER_FILE_NAME);
+        if (marker.exists()) {
+            // The background migration thread already finished (all builds have been already loaded at least once),
+            // so this is a belated load on a run that was not migrated in the main process (for some reason).
+            // Let's save it now.
+            run.save();
+        }
+    }
+
    /**
     * Exception to indicate a failure to migrate the IDStore.
     */
@@ -136,8 +150,16 @@ public void run() {
                         // touch something in the build just to force loading incase it gets more lazy in the future.
                         Object r = iterator.next();
                         if (r != null && r instanceof Run) {
-                            ((Run)r).getAllActions();
+                            Run run = ((Run) r);
+                            run.getAllActions();
+                            try {
+                                // Save the run here so its storage is updated (after being migrated in Id.onLoad(Run))
+                                run.save();
+                            } catch (IOException e) {
+                                LOGGER.log(Level.WARNING, String.format("Can not save build %s on job %s", run.getNumber(), job.getName()), e);
+                            }
                         }
+
                         migratedBuilds++;
                     }
                 }

src/main/java/org/jenkinsci/plugins/uniqueid/impl/RunIdStore.java

@@ -30,7 +30,7 @@ public void remove(Run run) throws IOException {
         List<Id> ids = run.getActions(Id.class);
         if (!ids.isEmpty()) {
             actionList.removeAll(ids);
-            run.save();
+            IdStoreMigratorV1ToV2.saveIfNeeded(run);
         }
     }
 

src/test/java/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2Test.java

@@ -43,7 +43,7 @@ public class IdStoreMigratorV1ToV2Test {
     public void testMigration() throws Exception {
         Jenkins jenkins = jenkinsRule.jenkins;
         assertThat("All jobs loaded correctly", jenkins.getAllItems(), hasSize(4));
-        
+
         Folder folderNoID = jenkins.getItem("folderNoID", jenkins, Folder.class);
         Folder folderWithID = jenkins.getItem("folderWithID", jenkins, Folder.class);
 
@@ -54,7 +54,7 @@ public void testMigration() throws Exception {
         assertThat(folderWithID, notNullValue());
         assertThat(jobNoID, notNullValue());
         assertThat(jobWithID, notNullValue());
-        
+
         checkID(folderNoID, null);
         checkID(folderWithID, "YzUxN2JiZTYtNGVhZS00NDQxLTg5NT");
 
@@ -66,10 +66,23 @@ public void testMigration() throws Exception {
         checkID(jobNoID.getBuildByNumber(2), null);
 
         // build 1 had no id so its generated on the fly from the parent
-        checkID(jobWithID.getBuildByNumber(1), "ZGQxMDNhYzUtMTJlOC00YTc4LTgzOT_" + jobWithID.getBuildByNumber(1).getId()); 
+        checkID(jobWithID.getBuildByNumber(1), "ZGQxMDNhYzUtMTJlOC00YTc4LTgzOT_" + jobWithID.getBuildByNumber(1).getId());
         checkID(jobWithID.getBuildByNumber(2), "NGQ0ODM2NjktZGM0OS00MjdkLWE3NT");
     }
-
+
+    @Test
+    @Issue("JENKINS-30424")
+    @LocalData
+    public void testMigrationLongLoadingBuilds() throws Exception {
+        Jenkins jenkins = jenkinsRule.jenkins;
+        assertThat("All jobs loaded correctly", jenkins.getAllItems(), hasSize(2));
+
+        Job jobWithID = jenkins.getItemByFullName("folderWithID/jobWithID", Job.class);
+        // Build #2 has a LongLoadingAction which simulates an action that blocks in the onLoad method
+        // This kind on delays causes issues on startup with version 2.1.0 (see issue linked to this test)
+        checkID(jobWithID.getBuildByNumber(2), "NGQ0ODM2NjktZGM0OS00MjdkLWE3NT");
+    }
+
     @Test
     @Issue("JENKINS-28883")
     public void migrateUnsupportedType() throws Exception {

src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingAction.java

@@ -0,0 +1,54 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2015, 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 org.jenkinsci.plugins.uniqueid.impl;
+
+import hudson.model.Run;
+import jenkins.model.RunAction2;
+
+public class LongLoadingAction implements RunAction2 {
+
+    public String getIconFileName() {
+        return null;
+    }
+
+    public String getDisplayName() {
+        return null;
+    }
+
+    public String getUrlName() {
+        return null;
+    }
+
+    public void onAttached(Run<?, ?> r) {
+    }
+
+    public void onLoad(Run<?, ?> r) {
+        try {
+            Thread.sleep(1000);
+        } catch (InterruptedException e) {
+            e.printStackTrace();
+        }
+    }
+
+}

src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingBuild.java

@@ -0,0 +1,60 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2015, 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 org.jenkinsci.plugins.uniqueid.impl;
+
+import java.io.File;
+import java.io.IOException;
+
+import hudson.model.Build;
+
+public class LongLoadingBuild extends Build<LongLoadingProject, LongLoadingBuild> {
+
+    private String check = null;
+
+    public LongLoadingBuild(LongLoadingProject project) throws IOException {
+        super(project);
+    }
+
+    public LongLoadingBuild(LongLoadingProject project, File buildDir) throws IOException {
+        super(project, buildDir);
+    }
+
+    @Override
+    public synchronized void save() throws IOException {
+        // Will produce a null pointer exception if called before onLoad finished.
+        check.toString();
+        super.save();
+    }
+
+    @Override
+    protected void onLoad() {
+        super.onLoad();
+        check = "Checked!";
+    }
+
+    @Override
+    public void run() {
+    }
+
+}

src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingProject.java

@@ -0,0 +1,59 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2015, 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 org.jenkinsci.plugins.uniqueid.impl;
+
+import org.jvnet.hudson.test.TestExtension;
+import hudson.model.ItemGroup;
+import hudson.model.Messages;
+import hudson.model.Project;
+import hudson.model.TopLevelItem;
+import hudson.model.TopLevelItemDescriptor;
+import jenkins.model.Jenkins;
+
+public class LongLoadingProject extends Project<LongLoadingProject, LongLoadingBuild> implements TopLevelItem {
+
+    public LongLoadingProject(ItemGroup parent, String name) {
+        super(parent, name);
+    }
+
+    @Override
+    protected Class<LongLoadingBuild> getBuildClass() {
+        return LongLoadingBuild.class;
+    }
+
+    @TestExtension
+    public static final class DescriptorImpl extends TopLevelItemDescriptor {
+        public String getDisplayName() {
+            return Messages.FreeStyleProject_DisplayName();
+        }
+
+        public LongLoadingProject newInstance(ItemGroup parent, String name) {
+            return new LongLoadingProject(parent,name);
+        }
+    }
+
+    public TopLevelItemDescriptor getDescriptor() {
+        return (DescriptorImpl) Jenkins.getInstance().getDescriptorOrDie(LongLoadingProject.class);
+    }
+}