diff --git a/src/main/java/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2.java b/src/main/java/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2.java index 9d347d0..166ef90 100644 --- a/src/main/java/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2.java +++ b/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++; } } diff --git a/src/main/java/org/jenkinsci/plugins/uniqueid/impl/RunIdStore.java b/src/main/java/org/jenkinsci/plugins/uniqueid/impl/RunIdStore.java index b1f4eb0..b76f39e 100644 --- a/src/main/java/org/jenkinsci/plugins/uniqueid/impl/RunIdStore.java +++ b/src/main/java/org/jenkinsci/plugins/uniqueid/impl/RunIdStore.java @@ -30,7 +30,7 @@ public void remove(Run run) throws IOException { List ids = run.getActions(Id.class); if (!ids.isEmpty()) { actionList.removeAll(ids); - run.save(); + IdStoreMigratorV1ToV2.saveIfNeeded(run); } } diff --git a/src/test/java/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2Test.java b/src/test/java/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2Test.java index 732c36f..daca2f7 100644 --- a/src/test/java/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2Test.java +++ b/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 { diff --git a/src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingAction.java b/src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingAction.java new file mode 100644 index 0000000..f8ac3ad --- /dev/null +++ b/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(); + } + } + +} diff --git a/src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingBuild.java b/src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingBuild.java new file mode 100644 index 0000000..047bf23 --- /dev/null +++ b/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 { + + 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() { + } + +} diff --git a/src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingProject.java b/src/test/java/org/jenkinsci/plugins/uniqueid/impl/LongLoadingProject.java new file mode 100644 index 0000000..f8a0938 --- /dev/null +++ b/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 implements TopLevelItem { + + public LongLoadingProject(ItemGroup parent, String name) { + super(parent, name); + } + + @Override + protected Class 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); + } +} diff --git a/src/test/resources/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2Test/testMigrationLongLoadingBuilds.zip b/src/test/resources/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2Test/testMigrationLongLoadingBuilds.zip new file mode 100644 index 0000000..cbecbc2 Binary files /dev/null and b/src/test/resources/org/jenkinsci/plugins/uniqueid/impl/IdStoreMigratorV1ToV2Test/testMigrationLongLoadingBuilds.zip differ