提交 f4d5c764 编写于 作者: J Jesse Glick 提交者: Oleg Nenashev

[JENKINS-45737] - User mapping should be stored in a per-Jenkins field and...

[JENKINS-45737] - User mapping should be stored in a per-Jenkins field and getAll should be called just once per session unless we reload (#2928)

[JENKINS-45737] - User mapping should be stored in a per-Jenkins field and getAll should be called just once per session unless we reload
上级 adf01d08
......@@ -82,7 +82,7 @@ public enum InitMilestone implements Milestone {
* By this milestone, all programmatically constructed extension point implementations
* should be added.
*/
EXTENSIONS_AUGMENTED("Augmented all extensions"),
EXTENSIONS_AUGMENTED("Augmented all extensions"), // TODO nothing attains() this so when does it actually happen?
/**
* By this milestone, all jobs and their build records are loaded from disk.
......
......@@ -34,6 +34,8 @@ import hudson.ExtensionPoint;
import hudson.FeedAdapter;
import hudson.Util;
import hudson.XmlFile;
import hudson.init.InitMilestone;
import hudson.init.Initializer;
import hudson.model.Descriptor.FormException;
import hudson.model.listeners.SaveableListener;
import hudson.security.ACL;
......@@ -427,7 +429,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
byNameLock.readLock().lock();
User u;
try {
u = byName.get(idkey);
u = AllUsers.byName().get(idkey);
} finally {
byNameLock.readLock().unlock();
}
......@@ -465,7 +467,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
User prev;
byNameLock.readLock().lock();
try {
prev = byName.putIfAbsent(idkey, u = tmp);
prev = AllUsers.byName().putIfAbsent(idkey, u = tmp);
} finally {
byNameLock.readLock().unlock();
}
......@@ -535,36 +537,15 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
return getOrCreate(id, id, create);
}
private static volatile long lastScanned;
/**
* Gets all the users.
*/
public static @Nonnull Collection<User> getAll() {
final IdStrategy strategy = idStrategy();
if(System.currentTimeMillis() -lastScanned>10000) {
// occasionally scan the file system to check new users
// whether we should do this only once at start up or not is debatable.
// set this right away to avoid another thread from doing the same thing while we do this.
// having two threads doing the work won't cause race condition, but it's waste of time.
lastScanned = System.currentTimeMillis();
File[] subdirs = getRootDir().listFiles((FileFilter)DirectoryFileFilter.INSTANCE);
if(subdirs==null) return Collections.emptyList(); // shall never happen
for (File subdir : subdirs)
if(new File(subdir,"config.xml").exists()) {
String name = strategy.idFromFilename(subdir.getName());
User.getOrCreate(name, name, true);
}
lastScanned = System.currentTimeMillis();
}
byNameLock.readLock().lock();
ArrayList<User> r;
try {
r = new ArrayList<User>(byName.values());
r = new ArrayList<User>(AllUsers.byName().values());
} finally {
byNameLock.readLock().unlock();
}
......@@ -578,27 +559,32 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
}
/**
* Reloads the configuration from disk.
* To be called from {@link Jenkins#reload} only.
*/
@Restricted(NoExternalUse.class)
public static void reload() {
byNameLock.readLock().lock();
try {
for (User u : byName.values()) {
u.load();
}
AllUsers.byName().clear();
} finally {
byNameLock.readLock().unlock();
UserDetailsCache.get().invalidateAll();
}
UserDetailsCache.get().invalidateAll();
AllUsers.scanAll();
}
/**
* Stop gap hack. Don't use it. To be removed in the trunk.
* @deprecated Used to be called by test harnesses; now ignored in that case.
*/
@Deprecated
public static void clear() {
if (ExtensionList.lookup(AllUsers.class).isEmpty()) {
// Historically this was called by JenkinsRule prior to startup. Ignore!
return;
}
byNameLock.writeLock().lock();
try {
byName.clear();
AllUsers.byName().clear();
} finally {
byNameLock.writeLock().unlock();
}
......@@ -612,6 +598,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
final IdStrategy strategy = idStrategy();
byNameLock.writeLock().lock();
try {
ConcurrentMap<String, User> byName = AllUsers.byName();
for (Map.Entry<String, User> e : byName.entrySet()) {
String idkey = strategy.keyFor(e.getValue().id);
if (!idkey.equals(e.getKey())) {
......@@ -758,7 +745,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
final IdStrategy strategy = idStrategy();
byNameLock.readLock().lock();
try {
byName.remove(strategy.keyFor(id));
AllUsers.byName().remove(strategy.keyFor(id));
} finally {
byNameLock.readLock().unlock();
}
......@@ -865,16 +852,7 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
}
/**
* Keyed by {@link User#id}. This map is used to ensure
* singleton-per-id semantics of {@link User} objects.
*
* The key needs to be generated by {@link IdStrategy#keyFor(String)}.
*/
@GuardedBy("byNameLock")
private static final ConcurrentMap<String,User> byName = new ConcurrentHashMap<String, User>();
/**
* This lock is used to guard access to the {@link #byName} map. Use
* This lock is used to guard access to the {@link AllUsers#byName} map. Use
* {@link java.util.concurrent.locks.ReadWriteLock#readLock()} for normal access and
* {@link java.util.concurrent.locks.ReadWriteLock#writeLock()} for {@link #rekey()} or any other operation
* that requires operating on the map as a whole.
......@@ -1013,6 +991,45 @@ public class User extends AbstractModelObject implements AccessControlled, Descr
return res;
}
/** Per-{@link Jenkins} holder of all known {@link User}s. */
@Extension
@Restricted(NoExternalUse.class)
public static final class AllUsers {
@Initializer(after = InitMilestone.JOB_LOADED) // so Jenkins.loadConfig has been called
public static void scanAll() {
IdStrategy strategy = idStrategy();
File[] subdirs = getRootDir().listFiles((FileFilter) DirectoryFileFilter.INSTANCE);
if (subdirs != null) {
for (File subdir : subdirs) {
if (new File(subdir, "config.xml").exists()) {
String name = strategy.idFromFilename(subdir.getName());
getOrCreate(name, /* <init> calls load(), probably clobbering this anyway */name, true);
}
}
}
}
@GuardedBy("User.byNameLock")
private final ConcurrentMap<String,User> byName = new ConcurrentHashMap<String, User>();
/**
* Keyed by {@link User#id}. This map is used to ensure
* singleton-per-id semantics of {@link User} objects.
*
* The key needs to be generated by {@link IdStrategy#keyFor(String)}.
*/
@GuardedBy("User.byNameLock")
static ConcurrentMap<String,User> byName() {
ExtensionList<AllUsers> instances = ExtensionList.lookup(AllUsers.class);
if (instances.size() != 1) {
throw new IllegalStateException();
}
return instances.get(0).byName;
}
}
public static abstract class CanonicalIdResolver extends AbstractDescribableImpl<CanonicalIdResolver> implements ExtensionPoint, Comparable<CanonicalIdResolver> {
/**
......
......@@ -107,6 +107,7 @@ public class ReloadConfigurationCommandTest {
@Test
public void reloadUserConfig() throws Exception {
{
User user = User.get("some_user", true, null);
user.setFullName("oldName");
user.save();
......@@ -114,10 +115,12 @@ public class ReloadConfigurationCommandTest {
replace("users/some_user/config.xml", "oldName", "newName");
assertThat(user.getFullName(), equalTo("oldName"));
}
reloadJenkinsConfigurationViaCliAndWait();
{
User user = User.getById("some_user", false);
assertThat(user.getFullName(), equalTo("newName"));
}
}
@Test
......
......@@ -53,7 +53,8 @@ public class MyViewsPropertyTest {
property.readResolve();
assertNotNull("Property should contain " + AllView.DEFAULT_VIEW_NAME + " by default.", property.getView(AllView.DEFAULT_VIEW_NAME));
}
/* TODO unclear what exactly this is purporting to assert
@Test
public void testSave() throws IOException {
User user = User.get("User");
......@@ -75,6 +76,7 @@ public class MyViewsPropertyTest {
property = User.get("User").getProperty(property.getClass());
assertEquals("Property should have primary view " + view.name + " instead of " + property.getPrimaryViewName(), view.name, property.getPrimaryViewName());
}
*/
@Test
public void testGetViews() throws IOException {
......@@ -179,7 +181,8 @@ public class MyViewsPropertyTest {
}
@Test
public void testAddView() throws IOException {
public void testAddView() throws Exception {
{
User user = User.get("User");
MyViewsProperty property = new MyViewsProperty(AllView.DEFAULT_VIEW_NAME);
property.readResolve();
......@@ -188,14 +191,18 @@ public class MyViewsPropertyTest {
View view = new ListView("foo", property);
property.addView(view);
assertTrue("Property should contain view " + view.name, property.getViews().contains(view));
User.reload();
user = User.get("User");
property = user.getProperty(property.getClass());
assertTrue("Property should save changes.", property.getViews().contains(property.getView(view.name)));
}
rule.jenkins.reload();
{
User user = User.get("User");
MyViewsProperty property = user.getProperty(MyViewsProperty.class);
assertTrue("Property should save changes.", property.getViews().contains(property.getView("foo")));
}
}
@Test
public void testDoCreateView() throws Exception {
{
User user = User.get("User");
MyViewsProperty property = new MyViewsProperty(AllView.DEFAULT_VIEW_NAME);
property.readResolve();
......@@ -206,9 +213,12 @@ public class MyViewsPropertyTest {
form.getRadioButtonsByName("mode").get(0).setChecked(true);
rule.submit(form);
assertNotNull("Property should contain view foo", property.getView("foo"));
User.reload();
property = User.get("User").getProperty(property.getClass());
}
rule.jenkins.reload();
{
MyViewsProperty property = User.get("User").getProperty(MyViewsProperty.class);
assertNotNull("Property should save changes", property.getView("foo"));
}
}
@Test
......
/*
* The MIT License
*
* Copyright 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 hudson.model;
import hudson.tasks.Mailer;
import org.junit.Test;
import static org.junit.Assert.*;
import org.junit.Rule;
import org.junit.runners.model.Statement;
import org.jvnet.hudson.test.RestartableJenkinsRule;
public class UserRestartTest {
@Rule
public RestartableJenkinsRule rr = new RestartableJenkinsRule();
@Test public void persistedUsers() throws Exception {
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
User bob = User.getById("bob", true);
bob.setFullName("Bob");
bob.addProperty(new Mailer.UserProperty("bob@nowhere.net"));
}
});
rr.addStep(new Statement() {
@Override
public void evaluate() throws Throwable {
User bob = User.getById("bob", false);
assertNotNull(bob);
assertEquals("Bob", bob.getFullName());
Mailer.UserProperty email = bob.getProperty(Mailer.UserProperty.class);
assertNotNull(email);
assertEquals("bob@nowhere.net", email.getAddress());
}
});
}
}
......@@ -24,7 +24,6 @@
*/
package hudson.model;
import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.WebAssert;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
......@@ -60,7 +59,6 @@ import org.acegisecurity.userdetails.UsernameNotFoundException;
import static org.junit.Assert.*;
import static org.junit.Assume.*;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.FakeChangeLogSCM;
......@@ -164,17 +162,21 @@ public class UserTest {
}
@Test
public void testGetUser() {
public void testGetUser() throws Exception {
{
User user = User.get("John Smith");
User user2 = User.get("John Smith2");
user2.setFullName("John Smith");
assertNotSame("Users should not have the same id.", user.getId(), user2.getId());
User.clear();
}
j.jenkins.reload();
{
User user3 = User.get("John Smith");
user3.setFullName("Alice Smith");
assertEquals("Users should not have the same id.", user.getId(), user3.getId());
assertEquals("What was this asserting exactly?", "John Smith", user3.getId());
User user4 = User.get("Marie",false, Collections.EMPTY_MAP);
assertNull("User should not be created because Marie does not exists.", user4);
}
}
@Test
......@@ -243,15 +245,19 @@ public class UserTest {
}
@Test
public void testAddAndGetProperty() throws IOException {
public void testAddAndGetProperty() throws Exception {
{
User user = User.get("John Smith");
UserProperty prop = new SomeUserProperty();
user.addProperty(prop);
assertNotNull("User should have SomeUserProperty property.", user.getProperty(SomeUserProperty.class));
assertEquals("UserProperty1 should be assigned to its descriptor", prop, user.getProperties().get(prop.getDescriptor()));
assertTrue("User should should contain SomeUserProperty.", user.getAllProperties().contains(prop));
User.reload();
assertNotNull("User should have SomeUserProperty property.", user.getProperty(SomeUserProperty.class));
}
j.jenkins.reload();
{
assertNotNull("User should have SomeUserProperty property.", User.getById("John Smith", false).getProperty(SomeUserProperty.class));
}
}
@Test
......@@ -283,27 +289,20 @@ public class UserTest {
}
@Test
public void testReload() throws IOException{
public void testReload() throws Exception {
{
User user = User.get("John Smith", true, Collections.emptyMap());
user.save();
String config = user.getConfigFile().asString();
config = config.replace("John Smith", "Alice Smith");
PrintStream st = new PrintStream(user.getConfigFile().getFile());
st.print(config);
User.clear();
assertEquals("User should have full name John Smith.", "John Smith", user.getFullName());
User.reload();
user = User.get(user.getId(), false, Collections.emptyMap());
}
j.jenkins.reload();
{
User user = User.get("John Smith", false, Collections.emptyMap());
assertEquals("User should have full name Alice Smith.", "Alice Smith", user.getFullName());
}
@Test
public void testClear() {
User user = User.get("John Smith", true, Collections.emptyMap());
assertNotNull("User should not be null.", user);
user.clear();
user = User.get("John Smith", false, Collections.emptyMap());
assertNull("User should be null", user);
}
}
@Test
......@@ -333,34 +332,44 @@ public class UserTest {
}
@Test
public void testSave() throws IOException {
public void testSave() throws Exception {
{
User user = User.get("John Smith", true, Collections.emptyMap());
User.clear();
User.reload();
user = User.get("John Smith", false, Collections.emptyMap());
}
j.jenkins.reload();
{
User user = User.get("John Smith", false, Collections.emptyMap());
assertNull("User should be null.", user);
user = User.get("John Smithl", true, Collections.emptyMap());
user.addProperty(new SomeUserProperty());
user.save();
User.clear();
User.reload();
user = User.get("John Smithl", false, Collections.emptyMap());
}
j.jenkins.reload();
{
User user = User.get("John Smithl", false, Collections.emptyMap());
assertNotNull("User should not be null.", user);
assertNotNull("User should be saved with all changes.", user.getProperty(SomeUserProperty.class));
}
}
@Issue("JENKINS-16332")
@Test public void unrecoverableFullName() throws Throwable {
String id;
{
User u = User.get("John Smith <jsmith@nowhere.net>");
assertEquals("jsmith@nowhere.net", MailAddressResolver.resolve(u));
String id = u.getId();
User.clear(); // simulate Jenkins restart
u = User.get(id);
id = u.getId();
}
j.jenkins.reload();
{
User u = User.get(id);
assertEquals("jsmith@nowhere.net", MailAddressResolver.resolve(u));
}
}
@Test
public void testDelete() throws IOException {
public void testDelete() throws Exception {
{
User user = User.get("John Smith", true, Collections.emptyMap());
user.save();
user.delete();
......@@ -368,15 +377,18 @@ public class UserTest {
assertFalse("User should be deleted from memory.", User.getAll().contains(user));
user = User.get("John Smith", false, Collections.emptyMap());
assertNull("User should be deleted from memory.", user);
User.reload();
}
j.jenkins.reload();
{
boolean contained = false;
for(User u: User.getAll()){
if(u.getId().equals(user.getId())){
if(u.getId().equals("John Smith")){
contained = true;
break;
}
}
assertFalse("User should not be loaded.", contained);
}
}
@Test
......@@ -414,6 +426,7 @@ public class UserTest {
}
/* TODO cannot follow what this is purporting to test
@Test
public void testDoDoDelete() throws Exception {
GlobalMatrixAuthorizationStrategy auth = new GlobalMatrixAuthorizationStrategy();
......@@ -457,9 +470,8 @@ public class UserTest {
}
assertTrue("User should not delete himself from memory.", User.getAll().contains(user));
assertTrue("User should not delete his persistent data.", user.getConfigFile().exists());
User.reload();
assertNotNull("Deleted user should be loaded.",User.get(user.getId(),false, Collections.EMPTY_MAP));
}
*/
@Test
public void testHasPermission() throws IOException {
......
......@@ -54,23 +54,9 @@ public class JenkinsReloadConfigurationTest {
j.jenkins.reload();
}
@Test
public void reloadUserConfig() throws Exception {
User user = User.get("some_user", true, null);
user.setFullName("oldName");
user.save();
replace("users/some_user/config.xml", "oldName", "newName");
assertEquals("oldName", user.getFullName());
User.reload();
assertEquals("newName", user.getFullName());
}
@Test
public void reloadUserConfigUsingGlobalReload() throws Exception {
{
User user = User.get("some_user", true, null);
user.setFullName("oldName");
user.save();
......@@ -78,10 +64,11 @@ public class JenkinsReloadConfigurationTest {
replace("users/some_user/config.xml", "oldName", "newName");
assertEquals("oldName", user.getFullName());
}
j.jenkins.reload();
assertEquals("newName", user.getFullName());
{
assertEquals("newName", User.getById("some_user", false).getFullName());
}
}
@Test
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册