Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-48707] - Use User#getById() in the code
User#get() is not effective from the performance PoV, because it tries to fetch users by full name. In all calls within the plugin it is not required, hence the update should achive some performance boost.
  • Loading branch information
oleg-nenashev committed Dec 25, 2017
1 parent 8164eab commit f6830fe
Show file tree
Hide file tree
Showing 15 changed files with 27 additions and 26 deletions.
Expand Up @@ -146,7 +146,7 @@ public boolean isOwnershipEnabled() {
*/
@CheckForNull
public User getPrimaryOwner() {
return ownershipEnabled ? User.get(primaryOwnerId, false, null) : null;
return ownershipEnabled ? User.getById(primaryOwnerId, false) : null;
}

/**
Expand Down
Expand Up @@ -237,7 +237,7 @@ public FormValidation doCheckUser(@QueryParameter String userId) {
return FormValidation.error("Field is empty. Field will be ignored");
}

User usr = User.get(userId, false, null);
User usr = User.getById(userId, false);
if (usr == null) {
return FormValidation.warning("User " + userId + " is not registered in Jenkins");
}
Expand Down
Expand Up @@ -90,8 +90,7 @@ public static boolean isUserExists(@Nonnull User user) {
}

public static boolean isUserExists(@Nonnull String userIdOrFullName) {
assert (userIdOrFullName != null);
return User.get(userIdOrFullName, false, null) != null;
return User.getById(userIdOrFullName, false) != null;
}

@Override
Expand Down
Expand Up @@ -58,7 +58,7 @@ public Authentication authenticate(Job<?, ?> job, Queue.Item item) {
if (!d.hasPrimaryOwner()) { // fallback to anonymous
return Jenkins.ANONYMOUS;
}
User owner = User.get(d.getPrimaryOwnerId(), false, Collections.emptyMap());
User owner = User.getById(d.getPrimaryOwnerId(), false);
if (owner == null) { // fallback to anonymous
return Jenkins.ANONYMOUS;
}
Expand Down
Expand Up @@ -44,6 +44,9 @@
import hudson.security.AccessControlled;
import org.jenkinsci.plugins.ownership.model.OwnershipHelperLocator;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

/**
* An abstract class for {@link RoleMacroExtension}s provided by the ownership plugin.
* @author Oleg Nenashev
Expand Down Expand Up @@ -101,13 +104,10 @@ public static OwnershipDescription getOwnership(RoleType type, AccessControlled
* @param macro Macro expression
* @param acceptSecondaryOwners {@code true} if secondary owners should be considered
* @return {@code true} if the macro provides a permission.
* Always {@code false} if the user is {@code null}.
*/
public static boolean hasPermission(User user, RoleType type, AccessControlled item,
Macro macro, boolean acceptSecondaryOwners) {
//TODO: implement
if (user == null) {
return false;
}
return getOwnership(type, item).isOwner(user, acceptSecondaryOwners);
public static boolean hasPermission(@CheckForNull User user, RoleType type, AccessControlled item,
Macro macro, boolean acceptSecondaryOwners) {
return user != null && getOwnership(type, item).isOwner(user, acceptSecondaryOwners);
}
}
Expand Up @@ -52,7 +52,7 @@ public String getDescription() {

@Override
public boolean hasPermission(String sid, Permission p, RoleType type, AccessControlled item, Macro macro) {
User user = User.get(sid, false, null);
User user = User.getById(sid, false);
return hasPermission(user, type, item, macro, true);
}
}
Expand Up @@ -55,7 +55,7 @@ public String getDescription() {

@Override
public boolean hasPermission(String sid, Permission p, RoleType type, AccessControlled item, Macro macro) {
User user = User.current();
User user = User.current();
return hasPermission(user, type, item, macro, true);
}
}
Expand Up @@ -51,7 +51,7 @@ public String getDescription() {

@Override
public boolean hasPermission(String sid, Permission p, RoleType type, AccessControlled item, Macro macro) {
User user = User.get(sid, false, null);
User user = User.getById(sid, false);
return hasPermission(user, type, item, macro, false);
}
}
Expand Up @@ -24,7 +24,6 @@
package com.synopsys.arc.jenkins.plugins.ownership.security.rolestrategy;

import com.synopsys.arc.jenkins.plugins.ownership.Messages;
import static com.synopsys.arc.jenkins.plugins.ownership.security.rolestrategy.AbstractOwnershipRoleMacro.hasPermission;
import com.synopsys.arc.jenkins.plugins.rolestrategy.Macro;
import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType;
import hudson.Extension;
Expand Down
Expand Up @@ -65,7 +65,7 @@ public class HTMLFormatter {
}

public static @Nonnull String formatUserURI(@Nonnull String userId, boolean useLongFormat) {
User usr = User.get(userId, false, null);
User usr = User.getById(userId, false);
if (usr != null) {
String userStr = useLongFormat
? usr.getDisplayName()
Expand Down
Expand Up @@ -49,7 +49,7 @@ public class UserStringFormatter {
}

public static @Nonnull String format(@Nonnull String userId) {
return format(User.get(userId, false, null));
return format(User.getById(userId, false));
}

public static @Nonnull String formatShort(@CheckForNull String userId) {
Expand All @@ -63,7 +63,7 @@ public class UserStringFormatter {
* @since 0.2
*/
public static @CheckForNull String formatEmail(@Nonnull String userId) {
return formatEmail(User.get(userId, false, null));
return formatEmail(User.getById(userId, false));
}

public static @CheckForNull String formatEmail(@CheckForNull User user) {
Expand Down
Expand Up @@ -60,7 +60,7 @@ public UserWrapper(@Nonnull String userMacro) {
this.macro = userMacro;
} else {
this.isUser = true;
this.user = User.get(userMacro, false, null);
this.user = User.getById(userMacro, false);
// throw new UnsupportedOperationException("User macro must start with prefix '"+USER_MACRO_PREFIX+"'");
}

Expand All @@ -82,7 +82,7 @@ public boolean isUser() {
/**
* Gets id of the user (calls User.getId() or returns macro).
*
* @return
* @return ID or macro
*/
public String getId() {
return isUser ? user.getId() : macro;
Expand Down
Expand Up @@ -24,13 +24,17 @@
package com.synopsys.arc.jenkins.plugins.ownership.util.ui;

import hudson.Extension;
import hudson.RestrictedSince;
import hudson.Util;
import hudson.model.Describable;
import hudson.model.Descriptor;
import hudson.model.User;
import hudson.util.FormValidation;
import java.io.Serializable;
import javax.annotation.CheckForNull;

import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;

Expand Down Expand Up @@ -84,14 +88,15 @@ public static class DescriptorImpl extends Descriptor<UserSelector> {
public String getDisplayName() {
return "N/A";
}


@Restricted(NoExternalUse.class)
public FormValidation doCheckSelectedUserId(@QueryParameter String selectedUserId) {
selectedUserId = Util.fixEmptyAndTrim(selectedUserId);
if (selectedUserId == null) {
return FormValidation.error("Field is empty. Field will be ignored");
}

User usr = User.get(selectedUserId, false, null);
User usr = User.getById(selectedUserId, false);
if (usr == null) {
return FormValidation.warning("User " + selectedUserId + " is not registered in Jenkins");
}
Expand Down
Expand Up @@ -24,7 +24,6 @@
package org.jenkinsci.plugins.ownership.integrations.rolestrategy.macros;

import com.synopsys.arc.jenkins.plugins.ownership.security.rolestrategy.AbstractOwnershipRoleMacro;
import static com.synopsys.arc.jenkins.plugins.ownership.security.rolestrategy.AbstractOwnershipRoleMacro.hasPermission;
import com.synopsys.arc.jenkins.plugins.rolestrategy.Macro;
import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType;
import hudson.Extension;
Expand Down
Expand Up @@ -24,7 +24,6 @@
package org.jenkinsci.plugins.ownership.integrations.rolestrategy.macros;

import com.synopsys.arc.jenkins.plugins.ownership.security.rolestrategy.AbstractOwnershipRoleMacro;
import static com.synopsys.arc.jenkins.plugins.ownership.security.rolestrategy.AbstractOwnershipRoleMacro.hasPermission;
import com.synopsys.arc.jenkins.plugins.rolestrategy.Macro;
import com.synopsys.arc.jenkins.plugins.rolestrategy.RoleType;
import hudson.Extension;
Expand Down Expand Up @@ -53,7 +52,7 @@ public String getDescription() {

@Override
public boolean hasPermission(String sid, Permission p, RoleType type, AccessControlled item, Macro macro) {
User user = User.current();
User user = User.current();
return hasPermission(user, type, item, macro, false);
}
}

0 comments on commit f6830fe

Please sign in to comment.