Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reduce usages of StringUtils #6270

Merged
merged 1 commit into from
Feb 14, 2022
Merged

Reduce usages of StringUtils #6270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/model/AllView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.servlet.ServletException;
import jenkins.util.SystemProperties;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.StaplerRequest;
Expand Down Expand Up @@ -146,7 +146,7 @@ public static String migrateLegacyPrimaryAllViewLocalizedName(@NonNull List<View
// name conflict, we cannot rename the all view anyway
return primaryView;
}
if (StringUtils.equals(v.getViewName(), primaryView)) {
if (Objects.equals(v.getViewName(), primaryView)) {
if (v instanceof AllView) {
allView = (AllView) v;
} else {
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/model/ChoiceParameterDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;
import net.sf.json.JSONObject;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand All @@ -39,7 +38,7 @@ public class ChoiceParameterDefinition extends SimpleParameterDefinition {

public static boolean areValidChoices(@NonNull String choices) {
String strippedChoices = choices.trim();
return !StringUtils.isEmpty(strippedChoices) && strippedChoices.split(CHOICES_DELIMITER).length > 0;
return strippedChoices != null && !strippedChoices.isEmpty() && strippedChoices.split(CHOICES_DELIMITER).length > 0;
}

public ChoiceParameterDefinition(@NonNull String name, @NonNull String choices, @CheckForNull String description) {
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/model/FileParameterValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import org.apache.commons.fileupload.util.FileItemHeadersImpl;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
Expand Down Expand Up @@ -158,7 +157,7 @@ public BuildWrapper createBuildWrapper(AbstractBuild<?, ?> build) {
@SuppressFBWarnings(value = {"FILE_UPLOAD_FILENAME", "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"}, justification = "TODO needs triage")
@Override
public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException {
if (!StringUtils.isEmpty(location) && !StringUtils.isEmpty(file.getName())) {
if (location != null && !location.isEmpty() && file.getName() != null && !file.getName().isEmpty()) {
listener.getLogger().println("Copying file to " + location);
FilePath ws = build.getWorkspace();
if (ws == null) {
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/model/Items.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import jenkins.model.Jenkins;
import jenkins.util.MemoryReductionUtil;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.StringUtils;
import org.springframework.security.core.Authentication;

/**
Expand Down Expand Up @@ -236,7 +235,7 @@ public static <T extends Item> List<T> fromNameList(ItemGroup context, @NonNull
StringTokenizer tokens = new StringTokenizer(list, ",");
while (tokens.hasMoreTokens()) {
String fullName = tokens.nextToken().trim();
if (StringUtils.isNotEmpty(fullName)) {
if (fullName != null && !fullName.isEmpty()) {
Copy link
Member

@timja timja Feb 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we create our own method for this, it was easier to read before this imo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guava has a Strings#isNullOrEmpty that is at least named correctly, but the problem with all such methods is that they encourage a sloppy programming style of overly defensive programming. Even the Guava version appears to be a concession, with warnings to avoid it:

Consider normalizing your string references with nullToEmpty(java.lang.String). If you do, you can use String.isEmpty() instead of this method, and you won't need special null-safe forms of methods like String.toUpperCase(java.util.Locale) either.

I think such methods are better avoided. Readable APIs encourage the use of a given programming paradigm, and in this case the paradigm is not one that I think should be encouraged.

Copy link
Member

@daniel-beck daniel-beck Feb 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Util#fixEmpty would allow

Suggested change
if (fullName != null && !fullName.isEmpty()) {
if (Util.fixEmpty(fullName) != null) {

(+ import in this file)

Copy link
Member

@olamy olamy Feb 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with @timja it was definitely easier to read.

T item = jenkins.getItem(fullName, context, type);
if (item != null)
r.add(item);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/hudson/model/Slave.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public void setUserId(String userId){
}

public ComputerLauncher getLauncher() {
if (launcher == null && !StringUtils.isEmpty(agentCommand)) {
if (launcher == null && agentCommand != null && !agentCommand.isEmpty()) {
try {
launcher = (ComputerLauncher) Jenkins.get().getPluginManager().uberClassLoader.loadClass("hudson.slaves.CommandLauncher").getConstructor(String.class, EnvVars.class).newInstance(agentCommand, null);
agentCommand = null;
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/hudson/model/ViewDescriptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import hudson.views.ViewJobFilter;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
import jenkins.model.DirectlyModifiableTopLevelItemGroup;
import jenkins.model.Jenkins;
import org.apache.commons.lang.StringUtils;
Expand Down Expand Up @@ -147,7 +148,7 @@ protected FormValidation checkDisplayName(@NonNull View view, @CheckForNull Stri
if (v.getViewName().equals(view.getViewName())) {
continue;
}
if (StringUtils.equals(v.getDisplayName(), value)) {
if (Objects.equals(v.getDisplayName(), value)) {
return FormValidation.warning(Messages.View_DisplayNameNotUniqueWarning(value));
}
}
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/hudson/security/HudsonAuthenticationEntryPoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.springframework.security.authentication.InsufficientAuthenticationException;
Expand Down Expand Up @@ -81,7 +80,7 @@ public void commence(HttpServletRequest req, HttpServletResponse rsp, Authentica
} else {
// give the opportunity to include the target URL
String uriFrom = req.getRequestURI();
if (!StringUtils.isEmpty(req.getQueryString())) uriFrom += "?" + req.getQueryString();
if (req.getQueryString() != null && !req.getQueryString().isEmpty()) uriFrom += "?" + req.getQueryString();
String loginForm = req.getContextPath() + loginFormUrl;
loginForm = MessageFormat.format(loginForm, URLEncoder.encode(uriFrom, "UTF-8"));
req.setAttribute("loginForm", loginForm);
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/jenkins/model/IdStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@
import java.io.Serializable;
import java.util.Comparator;
import java.util.Locale;
import java.util.Objects;
import java.util.function.Function;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.ProtectedExternally;
Expand Down Expand Up @@ -253,7 +253,7 @@ private Character convertCapitalizedAscii(String encoded) {

@Override
public boolean equals(@NonNull String id1, @NonNull String id2) {
return StringUtils.equals(id1, id2);
return Objects.equals(id1, id2);
}

@Override
Expand Down Expand Up @@ -289,7 +289,7 @@ public CaseSensitiveEmailAddress() {}

@Override
public boolean equals(@NonNull String id1, @NonNull String id2) {
return StringUtils.equals(keyFor(id1), keyFor(id2));
return Objects.equals(keyFor(id1), keyFor(id2));
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/jenkins/model/ProjectNamingStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public PatternProjectNamingStrategy(String namePattern, String description, bool
public void checkName(String name) {
if (StringUtils.isNotBlank(namePattern) && StringUtils.isNotBlank(name)) {
if (!Pattern.matches(namePattern, name)) {
throw new Failure(StringUtils.isEmpty(description) ?
throw new Failure(description == null || description.isEmpty() ?
Messages.Hudson_JobNameConventionNotApplyed(name, namePattern) :
description);
}
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/jenkins/mvn/FilePathGlobalSettingsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import hudson.FilePath;
import hudson.model.AbstractBuild;
import hudson.model.TaskListener;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.DataBoundConstructor;

/**
Expand All @@ -27,7 +26,7 @@ public String getPath() {

@Override
public FilePath supplySettings(AbstractBuild<?, ?> build, TaskListener listener) {
if (StringUtils.isEmpty(path)) {
if (path == null || path.isEmpty()) {
return null;
}

Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/jenkins/mvn/FilePathSettingsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import hudson.FilePath;
import hudson.model.AbstractBuild;
import hudson.model.TaskListener;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;

Expand All @@ -28,7 +27,7 @@ public String getPath() {

@Override
public FilePath supplySettings(AbstractBuild<?, ?> build, TaskListener listener) {
if (StringUtils.isEmpty(path)) {
if (path == null || path.isEmpty()) {
return null;
}

Expand Down
3 changes: 2 additions & 1 deletion test/src/test/java/hudson/cli/DisablePluginCommandTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import static hudson.cli.DisablePluginCommand.RETURN_CODE_NOT_DISABLED_DEPENDANTS;
import static hudson.cli.DisablePluginCommand.RETURN_CODE_NO_SUCH_PLUGIN;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.emptyOrNullString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertEquals;
Expand Down Expand Up @@ -293,7 +294,7 @@ public void quietModeEmptyOutputSucceed() {
assertPluginDisabled("depender");
assertPluginDisabled("mandatory-depender");

assertTrue("No log in quiet mode if all plugins disabled", StringUtils.isEmpty(result.stdout()));
assertThat("No log in quiet mode if all plugins disabled", result.stdout(), is(emptyOrNullString()));
}

/**
Expand Down
3 changes: 1 addition & 2 deletions test/src/test/java/hudson/model/QueueTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@
import org.apache.commons.fileupload.disk.DiskFileItemFactory;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.StringUtils;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.servlet.ServletHandler;
Expand Down Expand Up @@ -1283,7 +1282,7 @@ private String buildAndExtractTooltipAttribute() throws Exception {
DomElement buildQueue = page.getElementById("buildQueue");
DomNodeList<HtmlElement> anchors = buildQueue.getElementsByTagName("a");
HtmlAnchor anchorWithTooltip = (HtmlAnchor) anchors.stream()
.filter(a -> StringUtils.isNotEmpty(a.getAttribute("tooltip")))
.filter(a -> a.getAttribute("tooltip") != null && !a.getAttribute("tooltip").isEmpty())
.findFirst().orElseThrow(IllegalStateException::new);

String tooltip = anchorWithTooltip.getAttribute("tooltip");
Expand Down
3 changes: 1 addition & 2 deletions test/src/test/java/jenkins/security/Security637Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import org.apache.commons.lang.StringUtils;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -193,7 +192,7 @@ public void urlSafeDeserialization_inXStreamContext() throws Throwable {
URLJobProperty urlJobProperty = project.getProperty(URLJobProperty.class);
for (URL url : urlJobProperty.urlSet) {
URLStreamHandler handler = (URLStreamHandler) handlerField.get(url);
if (StringUtils.isEmpty(url.getHost())) {
if (url.getHost() == null || url.getHost().isEmpty()) {
assertThat(handler.getClass().getName(), not(containsString("SafeURLStreamHandler")));
} else {
assertThat(handler.getClass().getName(), containsString("SafeURLStreamHandler"));
Expand Down