Use Guava splitter instead of built in splitter.
Built in string splitter has some suprising behavior around how to deal
with empty strings and nulls.
Also stop hiding fields in integration test.
Change-Id: I239b33f2a6f7c759a3818329afbb78f1ee22efc9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
index 1a35009..a3c5e18 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/ConfigLoader.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.automerger;
import com.google.common.base.Joiner;
+import com.google.common.base.Splitter;
import com.google.common.base.Strings;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.api.GerritApi;
@@ -199,20 +200,24 @@
*/
public Set<String> getUpstreamBranches(String toBranch, String project)
throws ConfigInvalidException, RestApiException, IOException {
+ if (toBranch == null) {
+ throw new IllegalArgumentException("toBranch cannot be null");
+ }
Set<String> upstreamBranches = new HashSet<String>();
// List all subsections of automerger, split by :
Set<String> subsections = getConfig().getSubsections(pluginName);
for (String subsection : subsections) {
// Subsections are of the form "fromBranch:toBranch"
- String[] branchPair = subsection.split(Pattern.quote(BRANCH_DELIMITER));
- if (branchPair.length != 2) {
+ List<String> branchPair =
+ Splitter.on(BRANCH_DELIMITER).trimResults().omitEmptyStrings().splitToList(subsection);
+ if (branchPair.size() != 2) {
throw new ConfigInvalidException("Automerger config branch pair malformed: " + subsection);
}
- if (toBranch.equals(branchPair[1])) {
+ if (toBranch.equals(branchPair.get(1))) {
// If toBranch matches, check if project is in both their manifests
- Set<String> projectsInScope = getProjectsInScope(branchPair[0], branchPair[1]);
+ Set<String> projectsInScope = getProjectsInScope(branchPair.get(0), branchPair.get(1));
if (projectsInScope.contains(project)) {
- upstreamBranches.add(branchPair[0]);
+ upstreamBranches.add(branchPair.get(0));
}
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderIT.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderIT.java
index 85ff90c..d20f878 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderIT.java
@@ -27,6 +27,7 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -48,8 +49,8 @@
private ConfigLoader configLoader;
@Inject private AllProjectsName allProjectsName;
@Inject private PluginConfigFactory cfgFactory;
- @Inject private String canonicalWebUrl;
- @Inject private Provider<CurrentUser> user;
+ @Inject @CanonicalWebUrl String canonicalGerritWebUrl;
+ @Inject private Provider<CurrentUser> currentUser;
private Project.NameKey manifestNameKey;
@Test
@@ -241,7 +242,7 @@
@Test
public void getContextUserIdTest_noContextUser() throws Exception {
defaultSetup("automerger.config");
- assertThat(configLoader.getContextUserId()).isEqualTo(user.get().getAccountId());
+ assertThat(configLoader.getContextUserId()).isEqualTo(currentUser.get().getAccountId());
}
private void setupTestRepo(
@@ -279,6 +280,7 @@
private void loadConfig(String configFilename) throws Exception {
pushConfig(configFilename);
configLoader =
- new ConfigLoader(gApi, allProjectsName, "automerger", canonicalWebUrl, cfgFactory, user);
+ new ConfigLoader(
+ gApi, allProjectsName, "automerger", canonicalGerritWebUrl, cfgFactory, currentUser);
}
}
