Deal with NPE if no pattern is specified
Change-Id: I7974a4679afaf9371549923e7da2f2ae83353ae3
diff --git a/src/main/java/com/googlesource/gerrit/plugins/automerger/LoadedConfig.java b/src/main/java/com/googlesource/gerrit/plugins/automerger/LoadedConfig.java
index 058a879..5851f5a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/automerger/LoadedConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/automerger/LoadedConfig.java
@@ -44,8 +44,8 @@
global = Collections.emptyMap();
config = Collections.emptyMap();
defaultManifestInfo = Collections.emptyMap();
- blankMergePattern = Pattern.compile("");
- alwaysBlankMergePattern = Pattern.compile("");
+ blankMergePattern = null;
+ alwaysBlankMergePattern = null;
}
public LoadedConfig(
@@ -86,17 +86,18 @@
*/
public boolean isSkipMerge(String fromBranch, String toBranch, String commitMessage) {
// If regex matches always_blank_merge (DO NOT MERGE ANYWHERE), skip.
- if (alwaysBlankMergePattern.matches(commitMessage)) {
+ if (alwaysBlankMergePattern != null && alwaysBlankMergePattern.matches(commitMessage)) {
return true;
}
// If regex matches blank_merge (DO NOT MERGE), skip iff merge_all is false
- if (blankMergePattern.matches(commitMessage)) {
+ if (blankMergePattern != null && blankMergePattern.matches(commitMessage)) {
Map<String, Object> mergePairConfig = getMergeConfig(fromBranch, toBranch);
if (mergePairConfig != null) {
boolean isMergeAll = (boolean) mergePairConfig.getOrDefault("merge_all", false);
return !isMergeAll;
}
+ return true;
}
return false;
}
@@ -185,7 +186,11 @@
}
private Pattern getConfigPattern(String key) {
- Set<String> mergeStrings = new HashSet<>((List<String>) global.get(key));
- return Pattern.compile(Joiner.on("|").join(mergeStrings), Pattern.DOTALL);
+ Object patterns = global.get(key);
+ if (patterns != null) {
+ Set<String> mergeStrings = new HashSet<>((List<String>) patterns);
+ return Pattern.compile(Joiner.on("|").join(mergeStrings), Pattern.DOTALL);
+ }
+ return null;
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderTest.java b/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderTest.java
index 92c9391..4bf316a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/automerger/ConfigLoaderTest.java
@@ -109,6 +109,15 @@
}
@Test
+ public void isSkipMergeTest_alwaysBlankMergeNull() throws Exception {
+ mockFile("alternate_config.yaml", "tools/automerger", "master", "config.yaml");
+ loadConfig();
+ assertThat(
+ configLoader.isSkipMerge("master", "ds_two", "test test \n \n DO NOT MERGE ANYWHERE"))
+ .isFalse();
+ }
+
+ @Test
public void downstreamBranchesTest() throws Exception {
loadConfig();
Set<String> expectedBranches = new HashSet<String>();
diff --git a/src/test/resources/com/googlesource/gerrit/plugins/automerger/alternate_config.yaml b/src/test/resources/com/googlesource/gerrit/plugins/automerger/alternate_config.yaml
new file mode 100644
index 0000000..7c8c51a
--- /dev/null
+++ b/src/test/resources/com/googlesource/gerrit/plugins/automerger/alternate_config.yaml
@@ -0,0 +1,28 @@
+branches:
+ master:
+ ds_one:
+ add_projects:
+ - platform/added/project
+ ignore_projects:
+ - whoo
+ ds_two:
+ merge_all: true
+ add_projects:
+ - platform/added/project
+ ignore_projects:
+ - whoo
+ set_projects:
+ - platform/some/project
+ - platform/other/project
+ ds_two:
+ ds_three:
+ set_projects:
+ - platform/some/project
+global:
+ blank_merge:
+ - .*DO NOT MERGE.*
+ manifest:
+ file: default.xml
+ project: platform/manifest
+ ignore_projects:
+ - platform/ignore/me
