diff --git a/changelog.html b/changelog.html index d8754a7fe4e50343de9061b34a88d4ef8f3b44be..ea48a86389ea2a1d059c3e585d498e530352ca5d 100644 --- a/changelog.html +++ b/changelog.html @@ -61,6 +61,9 @@ Upcoming changes
  • Deadlock in OldDataMonitor. (issue 24358) +
  • + Failure to migrate legacy user records properly broke Jenkins. + (issue 24317) diff --git a/core/src/main/java/hudson/model/User.java b/core/src/main/java/hudson/model/User.java index ef078c2d210d7cc24da88be80786cc46af4b4bd1..41b9ff8548a1a0397cea3ad2e4f225395d7a7383 100644 --- a/core/src/main/java/hudson/model/User.java +++ b/core/src/main/java/hudson/model/User.java @@ -384,23 +384,26 @@ public class User extends AbstractModelObject implements AccessControlled, Descr // check for legacy users and migrate if safe to do so. File[] legacy = getLegacyConfigFilesFor(id); if (legacy != null && legacy.length > 0) { - for (File p : legacy) { - final XmlFile legacyXml = new XmlFile(XSTREAM, new File(p, "config.xml")); + for (File legacyUserDir : legacy) { + final XmlFile legacyXml = new XmlFile(XSTREAM, new File(legacyUserDir, "config.xml")); try { Object o = legacyXml.read(); if (o instanceof User) { - User tmp = (User) o; - if (idStrategy().equals(id, tmp.getId()) && !idStrategy().filenameOf(tmp.getId()) - .equals(p.getParentFile().getName())) { - if (!p.getParentFile().renameTo(configFile.getParentFile())) { - LOGGER.log(Level.FINE, "Could not migrate user record from {0} to {1}", - new Object[]{p.getParentFile(), configFile.getParentFile()}); + if (idStrategy().equals(id, legacyUserDir.getName()) && !idStrategy().filenameOf(legacyUserDir.getName()) + .equals(legacyUserDir.getName())) { + if (!legacyUserDir.renameTo(configFile.getParentFile())) { + LOGGER.log(Level.WARNING, "Failed to migrate user record from {0} to {1}", + new Object[]{legacyUserDir, configFile.getParentFile()}); } break; } + } else { + LOGGER.log(Level.FINE, "Unexpected object loaded from {0}: {1}", + new Object[]{ legacyUserDir, o }); } } catch (IOException e) { - // ignore + LOGGER.log(Level.FINE, String.format("Exception trying to load user from {0}: {1}", + new Object[]{ legacyUserDir, e.getMessage() }), e); } } } diff --git a/test/src/test/java/hudson/model/UserTest.java b/test/src/test/java/hudson/model/UserTest.java index 79883ba123bd0636a47d77c0ad5a12cab0db671e..41b3859af2cc6cbaf316791885e8bf37022934d8 100644 --- a/test/src/test/java/hudson/model/UserTest.java +++ b/test/src/test/java/hudson/model/UserTest.java @@ -33,8 +33,10 @@ import hudson.security.GlobalMatrixAuthorizationStrategy; import hudson.security.HudsonPrivateSecurityRealm; import hudson.security.Permission; import hudson.tasks.MailAddressResolver; +import java.io.File; import java.io.IOException; import java.io.PrintStream; +import java.util.Arrays; import java.util.Collections; import jenkins.model.IdStrategy; @@ -47,8 +49,10 @@ import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Bug; import org.jvnet.hudson.test.FakeChangeLogSCM; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.TestExtension; +import org.jvnet.hudson.test.recipes.LocalData; public class UserTest { @@ -220,6 +224,16 @@ public class UserTest { assertEquals(user2.getId(), User.idStrategy().idFromFilename(User.idStrategy().filenameOf(user2.getId()))); } + @Issue("JENKINS-24317") + @LocalData + @Test public void migration() throws Exception { + User bob = User.get("bob"); + assertEquals("Bob Smith", bob.getFullName()); + assertEquals("Bob Smith", User.get("Bob").getFullName()); + assertEquals("nonexistent", User.get("nonexistent").getFullName()); + assertEquals("[bob]", Arrays.toString(new File(j.jenkins.getRootDir(), "users").list())); + } + @Test public void testAddAndGetProperty() throws IOException { User user = User.get("John Smith"); diff --git a/test/src/test/resources/hudson/model/UserTest/migration.zip b/test/src/test/resources/hudson/model/UserTest/migration.zip new file mode 100644 index 0000000000000000000000000000000000000000..c5b73331c0d5c485b52dddc3b1d6a6636b50d8e2 Binary files /dev/null and b/test/src/test/resources/hudson/model/UserTest/migration.zip differ