ConfigEntry: Move source matching to the ConfigEntry
All the information to decide if the incoming project/ref matches a
conf is in the conf entry, keep the matching there.
This makes easier to add globs to choose branches in a
following change.
Change-Id: I0ebbd7da3691b545dc6cda0bdc517a9f40cf2d37
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/ConfigEntry.java b/java/com/googlesource/gerrit/plugins/supermanifest/ConfigEntry.java
index 681063f..d482917 100644
--- a/java/com/googlesource/gerrit/plugins/supermanifest/ConfigEntry.java
+++ b/java/com/googlesource/gerrit/plugins/supermanifest/ConfigEntry.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.entities.RefNames.REFS_HEADS;
import com.google.common.collect.ImmutableSet;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import java.net.URI;
import java.net.URISyntaxException;
@@ -30,6 +31,8 @@
import org.eclipse.jgit.transport.RefSpec.WildcardMode;
public class ConfigEntry {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
public static final String SECTION_NAME = "superproject";
final Project.NameKey srcRepoKey;
@@ -211,7 +214,27 @@
return destBranch;
}
- public boolean excludesRef(String refName) {
+ public boolean matchesSource(String project, String refName) {
+ if (!srcRepoKey.get().equals(project)) {
+ return false;
+ }
+
+ if (!(destBranch.equals("*") || srcRef.equals(refName))) {
+ return false;
+ }
+
+ if (destBranch.equals("*") && !refName.startsWith(REFS_HEADS)) {
+ return false;
+ }
+
+ if (excludesRef(refName)) {
+ logger.atInfo().log("Skipping %s: it matches exclude conditions.", refName);
+ return false;
+ }
+ return true;
+ }
+
+ private boolean excludesRef(String refName) {
for (String excluded : srcRefsExcluded) {
// ALLOW_MISMATCH allows '*' only in one side (source in this case)
RefSpec excludedSpec = new RefSpec(excluded, WildcardMode.ALLOW_MISMATCH);
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
index a1fe3e6..aa5f94b 100644
--- a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
+++ b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
@@ -14,7 +14,6 @@
package com.googlesource.gerrit.plugins.supermanifest;
-import static com.google.gerrit.entities.RefNames.REFS_HEADS;
import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.PLUGIN;
import com.google.common.annotations.VisibleForTesting;
@@ -68,6 +67,7 @@
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.api.errors.RefNotFoundException;
@@ -395,27 +395,9 @@
private List<ConfigEntry> findRelevantConfigs(
ImmutableSet<ConfigEntry> config, String project, String refName) {
- List<ConfigEntry> relevantConfigs = new ArrayList<>();
- for (ConfigEntry configEntry : config) {
- if (!configEntry.srcRepoKey.get().equals(project)) {
- continue;
- }
-
- if (!(configEntry.destBranch.equals("*") || configEntry.srcRef.equals(refName))) {
- continue;
- }
-
- if (configEntry.destBranch.equals("*") && !refName.startsWith(REFS_HEADS)) {
- continue;
- }
-
- if (configEntry.excludesRef(refName)) {
- info("Skipping %s: it matches exclude conditions.", refName);
- continue;
- }
- relevantConfigs.add(configEntry);
- }
- return relevantConfigs;
+ return config.stream()
+ .filter(c -> c.matchesSource(project, refName))
+ .collect(Collectors.toList());
}
private void updateForConfig(ConfigEntry configEntry, String refName)
diff --git a/javatests/com/googlesource/gerrit/plugins/supermanifest/ConfigEntryTest.java b/javatests/com/googlesource/gerrit/plugins/supermanifest/ConfigEntryTest.java
index 19e006d..6293f79 100644
--- a/javatests/com/googlesource/gerrit/plugins/supermanifest/ConfigEntryTest.java
+++ b/javatests/com/googlesource/gerrit/plugins/supermanifest/ConfigEntryTest.java
@@ -44,7 +44,10 @@
assertThat(entry.getSrcRepoKey()).isEqualTo(Project.nameKey("manifest"));
assertThat(entry.getSrcRef()).isEqualTo("refs/heads/nyc-src");
assertThat(entry.getXmlPath()).isEqualTo("default.xml");
- assertThat(entry.excludesRef("refs/heads/master")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/master")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/nyc-src")).isTrue();
+ assertThat(entry.matchesSource("manifest", "refs/heads/other")).isFalse();
+ assertThat(entry.matchesSource("otherproject", "refs/heads/nyc-src")).isFalse();
}
@Test
@@ -62,6 +65,9 @@
assertThat(entry.getSrcRepoKey()).isEqualTo(Project.nameKey("manifest"));
assertThat(entry.getSrcRef()).isEqualTo(""); // Ignored
assertThat(entry.getXmlPath()).isEqualTo("default.xml");
+ assertThat(entry.matchesSource("manifest", "refs/heads/a")).isTrue();
+ assertThat(entry.matchesSource("manifest", "refs/heads/b")).isTrue();
+ assertThat(entry.matchesSource("otherproject", "refs/heads/c")).isFalse();
}
@Test
@@ -122,7 +128,7 @@
}
@Test
- public void excluded() throws ConfigInvalidException {
+ public void matchesSource() throws ConfigInvalidException {
StringBuilder builder =
new StringBuilder(
getBasicConf(
@@ -136,11 +142,12 @@
cfg.fromText(builder.toString());
ConfigEntry entry = new ConfigEntry(cfg, "superproject:refs/heads/*");
- assertThat(entry.excludesRef("refs/heads/a")).isTrue();
- assertThat(entry.excludesRef("refs/heads/b")).isTrue();
- assertThat(entry.excludesRef("refs/heads/c")).isTrue();
- assertThat(entry.excludesRef("refs/tags/r1")).isFalse();
- assertThat(entry.excludesRef("refs/heads/aaa")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/d")).isTrue();
+ assertThat(entry.matchesSource("manifest", "refs/heads/nyc-src")).isTrue();
+ // Excluded
+ assertThat(entry.matchesSource("manifest", "refs/heads/a")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/tags/a")).isFalse();
+ assertThat(entry.matchesSource("other", "refs/heads/d")).isFalse();
}
@Test
@@ -158,11 +165,9 @@
cfg.fromText(builder.toString());
ConfigEntry entry = new ConfigEntry(cfg, "superproject:refs/heads/*");
- assertThat(entry.excludesRef("refs/heads/a")).isTrue();
- assertThat(entry.excludesRef("refs/heads/b")).isTrue();
- assertThat(entry.excludesRef("refs/heads/c")).isTrue();
- assertThat(entry.excludesRef("refs/tags/r1")).isFalse();
- assertThat(entry.excludesRef("refs/heads/aaa")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/a")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/b")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/c")).isFalse();
}
@Test
@@ -181,16 +186,15 @@
ConfigEntry entry = new ConfigEntry(cfg, "superproject:refs/heads/*");
// Excluded
- assertThat(entry.excludesRef("refs/heads/aa")).isTrue();
- assertThat(entry.excludesRef("refs/heads/a-something")).isTrue();
- assertThat(entry.excludesRef("refs/heads/b-release")).isTrue();
- assertThat(entry.excludesRef("refs/heads/bb-release")).isTrue();
+ assertThat(entry.matchesSource("manifest", "refs/heads/aa")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/a-something")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/b-release")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/bb-release")).isFalse();
// Non excluded
- assertThat(entry.excludesRef("refs/heads/a")).isFalse();
- assertThat(entry.excludesRef("refs/heads/master")).isFalse();
- assertThat(entry.excludesRef("refs/heads/c-release-c")).isFalse();
- assertThat(entry.excludesRef("refs/tags/aa")).isFalse();
+ assertThat(entry.matchesSource("manifest", "refs/heads/a")).isTrue();
+ assertThat(entry.matchesSource("manifest", "refs/heads/master")).isTrue();
+ assertThat(entry.matchesSource("manifest", "refs/heads/c-release-c")).isTrue();
}
private String getBasicConf(
