Prefer using Splitter to String.split

String.split(String) has surprising behaviour [1]:

String[] nothing = "".split(":"); // results in [""]
String[] bunchOfNothing = ":".split(":"); // results in []

More examples:

input  | input.split(":")  | Splitter.on(':').split(input)
=======|===================|==============================
""     | [""]              | [""]
":"    | []                | ["", ""]
":::"  | []                | ["", "", "", ""]
"a:::" | ["a"]             | ["a", "", "", ""]
":::b" | ["", "", "", "b"] | ["", "", "", "b"]

In addition using Splitter makes the code more readable as Splitter has
nicer methods that make it clearer which high-level operation should be
performed. E.g. in some places we can use
Splitter.on(CharMatcher.anyOf(something)) instead of matching on
patterns.

Tests and classes that are used by the GWT UI are not adapted.

[1] http://konigsberg.blogspot.com/2009/11/final-thoughts-java-puzzler-splitting.html

Change-Id: I6f141fb492e2fb94544089d794f245d0885f8649
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-02-09 16:49:35 +01:00
parent 2bf19ee6ee
commit 5174d2d95e
19 changed files with 99 additions and 75 deletions

View File

@@ -32,6 +32,8 @@ package com.google.gerrit.httpd.gitweb;
import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.CharMatcher;
import com.google.common.base.Splitter;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.Url;
@@ -71,6 +73,7 @@ import java.nio.file.Path;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -458,7 +461,7 @@ class GitwebServlet extends HttpServlet {
private static Map<String, String> getParameters(HttpServletRequest req) {
final Map<String, String> params = new HashMap<>();
for (String pair : req.getQueryString().split("[&;]")) {
for (String pair : Splitter.on(CharMatcher.anyOf("&;")).split(req.getQueryString())) {
final int eq = pair.indexOf('=');
if (0 < eq) {
String name = pair.substring(0, eq);
@@ -689,8 +692,8 @@ class GitwebServlet extends HttpServlet {
res.sendRedirect(value);
} else if ("Status".equalsIgnoreCase(key)) {
final String[] token = value.split(" ");
final int status = Integer.parseInt(token[0]);
final List<String> token = Splitter.on(' ').splitToList(value);
final int status = Integer.parseInt(token.get(0));
res.setStatus(status);
} else {

View File

@@ -190,6 +190,7 @@ public class RestApiServlet extends HttpServlet {
private static final int HEAP_EST_SIZE = 10 * 8 * 1024; // Presize 10 blocks.
private static final String PLAIN_TEXT = "text/plain";
private static final Pattern TYPE_SPLIT_PATTERN = Pattern.compile("[ ,;][ ,;]*");
/**
* Garbage prefix inserted before JSON output to prevent XSSI.
@@ -1259,7 +1260,7 @@ public class RestApiServlet extends HttpServlet {
if (given.startsWith(expect + ",")) {
return true;
}
for (String p : given.split("[ ,;][ ,;]*")) {
for (String p : Splitter.on(TYPE_SPLIT_PATTERN).split(given)) {
if (expect.equals(p)) {
return true;
}

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.pgm;
import com.google.common.base.Splitter;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.InstallAllPlugins;
@@ -49,13 +50,13 @@ public class Passwd extends SiteProgram {
private String password;
private void init() {
String[] varParts = sectionAndKey.split("\\.");
if (varParts.length != 2) {
List<String> varParts = Splitter.on('.').splitToList(sectionAndKey);
if (varParts.size() != 2) {
throw new IllegalArgumentException(
"Invalid name '" + sectionAndKey + "': expected section.key format");
}
section = varParts[0];
key = varParts[1];
section = varParts.get(0);
key = varParts.get(1);
}
@Override

View File

@@ -18,6 +18,7 @@ import static com.google.gerrit.pgm.init.api.InitUtil.die;
import static com.google.gerrit.pgm.init.api.InitUtil.savePublic;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Splitter;
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.pgm.init.api.InitStep;
@@ -168,7 +169,7 @@ class UpgradeFrom2_0_x implements InitStep {
if (url.contains("?")) {
final int q = url.indexOf('?');
for (String pair : url.substring(q + 1).split("&")) {
for (String pair : Splitter.on('&').split(url.substring(q + 1))) {
final int eq = pair.indexOf('=');
if (0 < eq) {
return false;

View File

@@ -14,8 +14,10 @@
package com.google.gerrit.server.account;
import com.google.common.base.Splitter;
import com.google.gerrit.reviewdb.client.Account;
import java.io.Serializable;
import java.util.List;
import java.util.Objects;
/** An SSH key approved for use by an {@link Account}. */
@@ -83,9 +85,9 @@ public final class AccountSshKey {
private String getPublicKeyPart(int index, String defaultValue) {
String s = getSshPublicKey();
if (s != null && s.length() > 0) {
String[] parts = s.split(" ");
if (parts.length > index) {
return parts[index];
List<String> parts = Splitter.on(' ').splitToList(s);
if (parts.size() > index) {
return parts.get(index);
}
}
return defaultValue;

View File

@@ -15,11 +15,13 @@
package com.google.gerrit.server.account;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.gerrit.reviewdb.client.Account;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.regex.Pattern;
public class AuthorizedKeys {
public static final String FILE_NAME = "authorized_keys";
@@ -28,10 +30,12 @@ public class AuthorizedKeys {
@VisibleForTesting public static final String DELETED_KEY_COMMENT = "# DELETED";
private static final Pattern LINE_SPLIT_PATTERN = Pattern.compile("\\r?\\n");
public static List<Optional<AccountSshKey>> parse(Account.Id accountId, String s) {
List<Optional<AccountSshKey>> keys = new ArrayList<>();
int seq = 1;
for (String line : s.split("\\r?\\n")) {
for (String line : Splitter.on(LINE_SPLIT_PATTERN).split(s)) {
line = line.trim();
if (line.isEmpty()) {
continue;

View File

@@ -15,10 +15,12 @@
package com.google.gerrit.server.account;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.io.BaseEncoding;
import com.google.common.primitives.Ints;
import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
import java.util.List;
import org.apache.commons.codec.DecoderException;
import org.bouncycastle.crypto.generators.BCrypt;
import org.bouncycastle.util.Arrays;
@@ -46,12 +48,12 @@ public class HashedPassword {
throw new DecoderException("unrecognized algorithm");
}
String[] fields = encoded.split(":");
if (fields.length != 4) {
List<String> fields = Splitter.on(':').splitToList(encoded);
if (fields.size() != 4) {
throw new DecoderException("want 4 fields");
}
Integer cost = Ints.tryParse(fields[1]);
Integer cost = Ints.tryParse(fields.get(1));
if (cost == null) {
throw new DecoderException("cost parse failed");
}
@@ -60,11 +62,11 @@ public class HashedPassword {
throw new DecoderException("cost should be 4..31 inclusive, got " + cost);
}
byte[] salt = codec.decode(fields[2]);
byte[] salt = codec.decode(fields.get(2));
if (salt.length != 16) {
throw new DecoderException("salt should be 16 bytes, got " + salt.length);
}
return new HashedPassword(codec.decode(fields[3]), salt, cost);
return new HashedPassword(codec.decode(fields.get(3)), salt, cost);
}
private static byte[] hashPassword(String password, byte[] salt, int cost) {

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.args4j;
import com.google.common.base.Splitter;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
@@ -23,6 +24,7 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import java.util.List;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.OptionDef;
@@ -47,16 +49,16 @@ public class ChangeIdHandler extends OptionHandler<Change.Id> {
@Override
public final int parseArguments(Parameters params) throws CmdLineException {
final String token = params.getParameter(0);
final String[] tokens = token.split(",");
if (tokens.length != 3) {
final List<String> tokens = Splitter.on(',').splitToList(token);
if (tokens.size() != 3) {
throw new CmdLineException(
owner, "change should be specified as <project>,<branch>,<change-id>");
}
try {
final Change.Key key = Change.Key.parse(tokens[2]);
final Project.NameKey project = new Project.NameKey(tokens[0]);
final Branch.NameKey branch = new Branch.NameKey(project, tokens[1]);
final Change.Key key = Change.Key.parse(tokens.get(2));
final Project.NameKey project = new Project.NameKey(tokens.get(0));
final Branch.NameKey branch = new Branch.NameKey(project, tokens.get(1));
for (ChangeData cd : queryProvider.get().byBranchKey(branch, key)) {
setter.addValue(cd.getId());
return 1;

View File

@@ -154,6 +154,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
private static final String EXTENSION_PANELS = "extension-panels";
private static final String KEY_PANEL = "panel";
private static final Pattern EXCLUSIVE_PERMISSIONS_SPLIT_PATTERN = Pattern.compile("[, \t]{1,}");
private Project.NameKey projectName;
private Project project;
private AccountsSection accountsSection;
@@ -678,7 +680,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
AccessSection as = getAccessSection(refName, true);
for (String varName : rc.getStringList(ACCESS, refName, KEY_GROUP_PERMISSIONS)) {
for (String n : varName.split("[, \t]{1,}")) {
for (String n : Splitter.on(EXCLUSIVE_PERMISSIONS_SPLIT_PATTERN).split(varName)) {
n = convertLegacyPermission(n);
if (isPermission(n)) {
as.getPermission(n, true).setExclusiveGroup(true);

View File

@@ -20,7 +20,9 @@ import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
import static java.util.stream.Collectors.toList;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.PageLinks;
@@ -334,9 +336,7 @@ public class CommitValidators {
sb.append("ERROR: ").append(errMsg);
if (c.getFullMessage().indexOf(CHANGE_ID_PREFIX) >= 0) {
String[] lines = c.getFullMessage().trim().split("\n");
String lastLine = lines.length > 0 ? lines[lines.length - 1] : "";
String lastLine = Iterables.getLast(Splitter.on('\n').split(c.getFullMessage()), "");
if (lastLine.indexOf(CHANGE_ID_PREFIX) == -1) {
sb.append('\n');
sb.append('\n');

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server.mail.receive;
import static com.google.gerrit.server.mail.MetadataName.toFooterWithDelimiter;
import static com.google.gerrit.server.mail.MetadataName.toHeaderWithDelimiter;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.primitives.Ints;
import com.google.gerrit.server.mail.MailUtil;
@@ -62,7 +63,7 @@ public class MetadataParser {
// If the required fields were not yet found, continue to parse the text
if (!Strings.isNullOrEmpty(m.textContent())) {
String[] lines = m.textContent().replace("\r\n", "\n").split("\n");
Iterable<String> lines = Splitter.on('\n').split(m.textContent().replace("\r\n", "\n"));
extractFooters(lines, metadata, m);
if (metadata.hasRequiredFields()) {
return metadata;
@@ -72,7 +73,7 @@ public class MetadataParser {
// If the required fields were not yet found, continue to parse the HTML
// HTML footer are contained inside a <div> tag
if (!Strings.isNullOrEmpty(m.htmlContent())) {
String[] lines = m.htmlContent().replace("\r\n", "\n").split("</div>");
Iterable<String> lines = Splitter.on("</div>").split(m.htmlContent().replace("\r\n", "\n"));
extractFooters(lines, metadata, m);
if (metadata.hasRequiredFields()) {
return metadata;
@@ -82,7 +83,7 @@ public class MetadataParser {
return metadata;
}
private static void extractFooters(String[] lines, MailMetadata metadata, MailMessage m) {
private static void extractFooters(Iterable<String> lines, MailMetadata metadata, MailMessage m) {
for (String line : lines) {
if (metadata.changeNumber == null && line.contains(MetadataName.CHANGE_NUMBER)) {
metadata.changeNumber =

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.mail.receive;
import com.google.common.base.Splitter;
import com.google.common.collect.Iterables;
import com.google.gerrit.reviewdb.client.Comment;
import java.util.List;
@@ -37,34 +38,34 @@ public class ParserUtil {
*/
public static String trimQuotation(String comment) {
StringJoiner j = new StringJoiner("\n");
String[] lines = comment.split("\n");
for (int i = 0; i < lines.length - 2; i++) {
j.add(lines[i]);
List<String> lines = Splitter.on('\n').splitToList(comment);
for (int i = 0; i < lines.size() - 2; i++) {
j.add(lines.get(i));
}
// Check if the last line contains the full quotation pattern (date + email)
String lastLine = lines[lines.length - 1];
String lastLine = lines.get(lines.size() - 1);
if (containsQuotationPattern(lastLine)) {
if (lines.length > 1) {
j.add(lines[lines.length - 2]);
if (lines.size() > 1) {
j.add(lines.get(lines.size() - 2));
}
return j.toString().trim();
}
// Check if the second last line + the last line contain the full quotation pattern. This is
// necessary, as the quotation line can be split across the last two lines if it gets too long.
if (lines.length > 1) {
String lastLines = lines[lines.length - 2] + lastLine;
if (lines.size() > 1) {
String lastLines = lines.get(lines.size() - 2) + lastLine;
if (containsQuotationPattern(lastLines)) {
return j.toString().trim();
}
}
// Add the last two lines
if (lines.length > 1) {
j.add(lines[lines.length - 2]);
if (lines.size() > 1) {
j.add(lines.get(lines.size() - 2));
}
j.add(lines[lines.length - 1]);
j.add(lines.get(lines.size() - 1));
return j.toString().trim();
}

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.mail.receive;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.common.collect.Iterators;
import com.google.common.collect.PeekingIterator;
@@ -63,11 +64,10 @@ public class TextParser {
PeekingIterator<Comment> iter = Iterators.peekingIterator(comments.iterator());
String[] lines = body.split("\n");
MailComment currentComment = null;
String lastEncounteredFileName = null;
Comment lastEncounteredComment = null;
for (String line : lines) {
for (String line : Splitter.on('\n').split(body)) {
if (line.equals(">")) {
// Skip empty lines
continue;

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server.mail.send;
import static com.google.common.base.Strings.isNullOrEmpty;
import com.google.common.base.Splitter;
import com.google.gerrit.common.Nullable;
import java.util.ArrayList;
import java.util.Collections;
@@ -52,7 +53,7 @@ public class CommentFormatter {
}
List<Block> result = new ArrayList<>();
for (String p : source.split("\n\n")) {
for (String p : Splitter.on("\n\n").split(source)) {
if (isQuote(p)) {
result.add(makeQuote(p));
} else if (isPreFormat(p)) {
@@ -96,7 +97,7 @@ public class CommentFormatter {
boolean inList = false;
boolean inParagraph = false;
for (String line : p.split("\n")) {
for (String line : Splitter.on('\n').split(p)) {
if (line.startsWith("-") || line.startsWith("*")) {
// The next line looks like a list item. If not building a list already,
// then create one. Remove the list item marker (* or -) from the line.

View File

@@ -21,6 +21,7 @@ import static java.util.stream.Collectors.toSet;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Enums;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
@@ -526,9 +527,9 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
}
// for plugins the value will be operandName_pluginName
String[] names = value.split("_");
if (names.length == 2) {
ChangeHasOperandFactory op = args.hasOperands.get(names[1], names[0]);
List<String> names = Lists.newArrayList(Splitter.on('_').split(value));
if (names.size() == 2) {
ChangeHasOperandFactory op = args.hasOperands.get(names.get(1), names.get(0));
if (op != null) {
return op.create(this);
}
@@ -729,12 +730,12 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
// Special case: votes by owners can be tracked with ",owner":
// label:Code-Review+2,owner
// label:Code-Review+2,user=owner
String[] splitReviewer = name.split(",", 2);
name = splitReviewer[0]; // remove all but the vote piece, e.g.'CodeReview=1'
List<String> splitReviewer = Lists.newArrayList(Splitter.on(',').limit(2).split(name));
name = splitReviewer.get(0); // remove all but the vote piece, e.g.'CodeReview=1'
if (splitReviewer.length == 2) {
if (splitReviewer.size() == 2) {
// process the user/group piece
PredicateArgs lblArgs = new PredicateArgs(splitReviewer[1]);
PredicateArgs lblArgs = new PredicateArgs(splitReviewer.get(1));
for (Map.Entry<String, String> pair : lblArgs.keyValue.entrySet()) {
if (pair.getKey().equalsIgnoreCase(ARG_ID_USER)) {

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
import com.google.common.base.Splitter;
import com.google.gerrit.index.query.QueryParseException;
import java.util.ArrayList;
import java.util.HashMap;
@@ -45,18 +46,16 @@ public class PredicateArgs {
positional = new ArrayList<>();
keyValue = new HashMap<>();
String[] splitArgs = args.split(",");
for (String arg : Splitter.on(',').split(args)) {
List<String> splitKeyValue = Splitter.on('=').splitToList(arg);
for (String arg : splitArgs) {
String[] splitKeyValue = arg.split("=");
if (splitKeyValue.length == 1) {
positional.add(splitKeyValue[0]);
} else if (splitKeyValue.length == 2) {
if (!keyValue.containsKey(splitKeyValue[0])) {
keyValue.put(splitKeyValue[0], splitKeyValue[1]);
if (splitKeyValue.size() == 1) {
positional.add(splitKeyValue.get(0));
} else if (splitKeyValue.size() == 2) {
if (!keyValue.containsKey(splitKeyValue.get(0))) {
keyValue.put(splitKeyValue.get(0), splitKeyValue.get(1));
} else {
throw new QueryParseException("Duplicate key " + splitKeyValue[0]);
throw new QueryParseException("Duplicate key " + splitKeyValue.get(0));
}
} else {
throw new QueryParseException("invalid arg " + arg);

View File

@@ -17,6 +17,8 @@ package com.google.gerrit.sshd;
import static com.google.gerrit.extensions.registration.PrivateInternals_DynamicTypes.registerInParentInjectors;
import static com.google.inject.Scopes.SINGLETON;
import com.google.common.base.CharMatcher;
import com.google.common.base.Splitter;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.server.DynamicOptions;
@@ -37,6 +39,7 @@ import com.google.inject.internal.UniqueAnnotations;
import com.google.inject.servlet.RequestScoped;
import java.net.SocketAddress;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import org.apache.sshd.server.CommandFactory;
@@ -108,10 +111,10 @@ public class SshModule extends LifecycleModule {
CommandName gerrit = Commands.named("gerrit");
for (Map.Entry<String, String> e : aliases.entrySet()) {
String name = e.getKey();
String[] dest = e.getValue().split("[ \\t]+");
CommandName cmd = Commands.named(dest[0]);
for (int i = 1; i < dest.length; i++) {
cmd = Commands.named(cmd, dest[i]);
List<String> dest = Splitter.on(CharMatcher.whitespace()).splitToList(e.getValue());
CommandName cmd = Commands.named(dest.get(0));
for (int i = 1; i < dest.size(); i++) {
cmd = Commands.named(cmd, dest.get(i));
}
bind(Commands.key(gerrit, name)).toProvider(new AliasCommandProvider(cmd));
}

View File

@@ -227,9 +227,9 @@ final class CreateProjectCommand extends SshCommand {
throws UnloggedFailure {
Map<String, Map<String, ConfigValue>> m = new HashMap<>();
for (String pluginConfigValue : pluginConfigValues) {
String[] s = pluginConfigValue.split("=");
String[] s2 = s[0].split("\\.");
if (s.length != 2 || s2.length != 2) {
List<String> s = Splitter.on('=').splitToList(pluginConfigValue);
List<String> s2 = Splitter.on('.').splitToList(s.get(0));
if (s.size() != 2 || s2.size() != 2) {
throw die(
"Invalid plugin config value '"
+ pluginConfigValue
@@ -237,14 +237,14 @@ final class CreateProjectCommand extends SshCommand {
+ " or '<plugin-name>.<parameter-name>=<value1,value2,...>'");
}
ConfigValue value = new ConfigValue();
String v = s[1];
String v = s.get(1);
if (v.contains(",")) {
value.values = Lists.newArrayList(Splitter.on(",").split(v));
value.values = Splitter.on(",").splitToList(v);
} else {
value.value = v;
}
String pluginName = s2[0];
String paramName = s2[1];
String pluginName = s2.get(0);
String paramName = s2.get(1);
Map<String, ConfigValue> l = m.get(pluginName);
if (l == null) {
l = new HashMap<>();

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.sshd.commands;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.change.ArchiveFormat;
@@ -142,8 +143,7 @@ public class UploadArchive extends AbstractGitCommand {
if (!s.startsWith(argCmd)) {
throw new Failure(1, "fatal: 'argument' token or flush expected, got " + s);
}
String[] parts = s.substring(argCmd.length()).split("=", 2);
for (String p : parts) {
for (String p : Splitter.on('=').limit(2).split(s.substring(argCmd.length()))) {
args.add(p);
}
}