Merge "Load additional plugin-specific rule base"
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/RuleBase.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/RuleBase.java
index 685a639..cfa70ac 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/RuleBase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/workflow/RuleBase.java
@@ -19,6 +19,7 @@
import java.util.Collection;
import com.google.common.collect.Lists;
+import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.config.SitePath;
import com.google.inject.Inject;
@@ -35,18 +36,23 @@
private static final Logger log = LoggerFactory.getLogger(RuleBase.class);
/**
- * File (relative to site) to load rules from
+ * File beginning (relative to site) to load rules from
*/
- private static final String ITS_CONFIG_FILE = "etc" + File.separatorChar +
- "its" + File.separator + "actions.config";
+ private static final String ITS_CONFIG_FILE_START = "etc" +
+ File.separatorChar + "its" + File.separator + "actions";
/**
- * The section for rules within {@link #ITS_CONFIG_FILE}
+ * File end to load rules from
+ */
+ private static final String ITS_CONFIG_FILE_END = ".config";
+
+ /**
+ * The section for rules within rulebases
*/
private static final String RULE_SECTION = "rule";
/**
- * The key for actions within {@link #ITS_CONFIG_FILE}
+ * The key for actions within rulebases
*/
private static final String ACTION_KEY = "action";
@@ -54,6 +60,7 @@
private final Rule.Factory ruleFactory;
private final Condition.Factory conditionFactory;
private final ActionRequest.Factory actionRequestFactory;
+ private final String pluginName;
private Collection<Rule> rules;
@@ -64,11 +71,13 @@
@Inject
public RuleBase(@SitePath File sitePath, Rule.Factory ruleFactory,
Condition.Factory conditionFactory,
- ActionRequest.Factory actionRequestFactory) {
+ ActionRequest.Factory actionRequestFactory,
+ @PluginName String pluginName) {
this.sitePath = sitePath;
this.ruleFactory = ruleFactory;
this.conditionFactory = conditionFactory;
this.actionRequestFactory = actionRequestFactory;
+ this.pluginName = pluginName;
reloadRules();
}
@@ -109,8 +118,6 @@
}
rules.add(rule);
}
- } else {
- log.warn("ITS actions configuration file (" + ruleFile + ") does not exist.");
}
}
@@ -138,8 +145,26 @@
}
// Add global rules
- File globalRuleFile = new File(sitePath, ITS_CONFIG_FILE);
+ File globalRuleFile = new File(sitePath, ITS_CONFIG_FILE_START +
+ ITS_CONFIG_FILE_END);
addRulesFromFile(globalRuleFile);
+
+ // Add its-specific rules
+ File itsSpecificRuleFile = new File(sitePath, ITS_CONFIG_FILE_START + "-" +
+ pluginName + ITS_CONFIG_FILE_END);
+ addRulesFromFile(itsSpecificRuleFile);
+
+ if (!globalRuleFile.exists() && !itsSpecificRuleFile.exists()) {
+ try {
+ log.warn("Neither global rule file "
+ + globalRuleFile.getCanonicalPath() + " nor Its specific rule file"
+ + itsSpecificRuleFile.getCanonicalPath() + " exist. Please configure "
+ + "rules.");
+ } catch (IOException e) {
+ log.warn("Neither global rule file nor Its specific rule files exist. "
+ + "Please configure rules.");
+ }
+ }
}
/**
diff --git a/src/main/resources/Documentation/config-rulebase-common.md b/src/main/resources/Documentation/config-rulebase-common.md
index 4712e31..1e79fa5 100644
--- a/src/main/resources/Documentation/config-rulebase-common.md
+++ b/src/main/resources/Documentation/config-rulebase-common.md
@@ -18,13 +18,16 @@
‘Resolved’”) on the ITS.
Actions on the ITS and conditions for the action to take place are
-configured through the rule base in `etc/its/actions.config` in the
-site directory. The rule base is a git config file, and may contain an
-arbitrary number of rules. Each rule can have an arbitrary number of
-conditions and actions. A rule fires all associated actions, once all
-of its conditions are met.
+configured through the rule bases in `etc/its/actions.config` (for global rules
+to be picked up by all configured ITS plugins) and
+`etc/its/actions-@PLUGIN@.config` (for rules to be picked up only by @PLUGIN@)
+in the site directory. A rule base is a git config file, and may contain an
+arbitrary number of rules. Each rule can have an arbitrary number of conditions
+and actions. A rule fires all associated actions, once all of its conditions are
+met.
-A simple `etc/its/actions.config` may look like
+A simple `etc/its/actions.config` (or
+`etc/its/actions-@PLUGIN@.config`) may look like
```
[rule "rule1"]
diff --git a/src/test/java/com/googlesource/gerrit/plugins/hooks/workflow/RuleBaseTest.java b/src/test/java/com/googlesource/gerrit/plugins/hooks/workflow/RuleBaseTest.java
index b8d20d9..ffadc1d 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/hooks/workflow/RuleBaseTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/hooks/workflow/RuleBaseTest.java
@@ -27,10 +27,12 @@
import org.eclipse.jgit.util.FileUtils;
import com.google.common.collect.Lists;
+import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.config.FactoryModule;
import com.google.gerrit.server.config.SitePath;
import com.google.inject.Guice;
import com.google.inject.Injector;
+
import com.googlesource.gerrit.plugins.hooks.testutil.LoggingMockingTestCase;
public class RuleBaseTest extends LoggingMockingTestCase {
@@ -43,12 +45,18 @@
private boolean cleanupSitePath;
+ private enum RuleBaseKind {
+ GLOBAL,
+ ITS,
+ FAULTY
+ }
+
public void testWarnNonExistingRuleBase() {
replayMocks();
createRuleBase();
- assertLogMessageContains("does not exist");
+ assertLogMessageContains("Neither global");
}
public void testEmptyRuleBase() throws IOException {
@@ -219,20 +227,20 @@
}
public void testWarnExistingFaultyNameRuleBaseFile() throws IOException {
- injectRuleBase("", true);
+ injectRuleBase("", RuleBaseKind.FAULTY);
replayMocks();
createRuleBase();
assertLogMessageContains("Please migrate"); // Migration warning for old name
- assertLogMessageContains("does not exist"); // For global rule file at new name
+ assertLogMessageContains("Neither global"); // For rule file at at usual places
}
public void testSimpleFaultyNameRuleBase() throws IOException {
injectRuleBase("[rule \"rule1\"]\n" +
"\tconditionA = value1\n" +
- "\taction = action1", true);
+ "\taction = action1", RuleBaseKind.FAULTY);
Rule rule1 = createMock(Rule.class);
expect(ruleFactory.create("rule1")).andReturn(rule1);
@@ -250,15 +258,39 @@
createRuleBase();
assertLogMessageContains("Please migrate"); // Migration warning for old name
- assertLogMessageContains("does not exist"); // For global rule file at new name
+ assertLogMessageContains("Neither global"); // For rule file at at usual places
}
- public void testGlobalRuleBaseFileAndFaultyNameFileAreLoaded() throws IOException {
+ public void testSimpleItsRuleBase() throws IOException {
injectRuleBase("[rule \"rule1\"]\n" +
- "\taction = action1", false);
+ "\tconditionA = value1\n" +
+ "\taction = action1", RuleBaseKind.ITS);
+
+ Rule rule1 = createMock(Rule.class);
+ expect(ruleFactory.create("rule1")).andReturn(rule1);
+
+ Condition condition1 = createMock(Condition.class);
+ expect(conditionFactory.create("conditionA", "value1")).andReturn(condition1);
+ rule1.addCondition(condition1);
+
+ ActionRequest actionRequest1 = createMock(ActionRequest.class);
+ expect(actionRequestFactory.create("action1")).andReturn(actionRequest1);
+ rule1.addActionRequest(actionRequest1);
+
+ replayMocks();
+
+ createRuleBase();
+ }
+
+ public void testAllRuleBaseFilesAreLoaded() throws IOException {
+ injectRuleBase("[rule \"rule1\"]\n" +
+ "\taction = action1", RuleBaseKind.FAULTY);
injectRuleBase("[rule \"rule2\"]\n" +
- "\taction = action2", true);
+ "\taction = action2", RuleBaseKind.GLOBAL);
+
+ injectRuleBase("[rule \"rule3\"]\n" +
+ "\taction = action3", RuleBaseKind.ITS);
Collection<Property> properties = Collections.emptySet();
@@ -281,18 +313,30 @@
rule2.addActionRequest(actionRequest2);
List<ActionRequest> rule2Match = Lists.newArrayListWithCapacity(1);
- rule1Match.add(actionRequest2);
+ rule2Match.add(actionRequest2);
expect(rule2.actionRequestsFor(properties)).andReturn(rule2Match);
+ Rule rule3 = createMock(Rule.class);
+ expect(ruleFactory.create("rule3")).andReturn(rule3);
+
+ ActionRequest actionRequest3 = createMock(ActionRequest.class);
+ expect(actionRequestFactory.create("action3")).andReturn(actionRequest3);
+ rule3.addActionRequest(actionRequest3);
+
+ List<ActionRequest> rule3Match = Lists.newArrayListWithCapacity(1);
+ rule3Match.add(actionRequest3);
+ expect(rule3.actionRequestsFor(properties)).andReturn(rule3Match);
+
replayMocks();
RuleBase ruleBase = createRuleBase();
Collection<ActionRequest> actual = ruleBase.actionRequestsFor(properties);
- List<ActionRequest> expected = Lists.newArrayListWithCapacity(2);
+ List<ActionRequest> expected = Lists.newArrayListWithCapacity(3);
expected.add(actionRequest1);
expected.add(actionRequest2);
+ expected.add(actionRequest3);
assertEquals("Matched actionRequests do not match", expected, actual);
@@ -304,12 +348,27 @@
}
private void injectRuleBase(String rules) throws IOException {
- injectRuleBase(rules, false);
+ injectRuleBase(rules, RuleBaseKind.GLOBAL);
}
- private void injectRuleBase(String rules, Boolean faultyName) throws IOException {
+ private void injectRuleBase(String rules, RuleBaseKind ruleBaseKind) throws IOException {
+ String baseName = "";
+ switch (ruleBaseKind) {
+ case GLOBAL:
+ baseName = "actions";
+ break;
+ case ITS:
+ baseName = "actions-ItsTestName";
+ break;
+ case FAULTY:
+ baseName = "action";
+ break;
+ default:
+ fail("Unknown ruleBaseKind");
+ }
File ruleBaseFile = new File(sitePath, "etc" + File.separatorChar + "its" +
- File.separator + "action" + (faultyName ? "" : "s") + ".config");
+ File.separator + baseName + ".config");
+
File ruleBaseParentFile = ruleBaseFile.getParentFile();
if (!ruleBaseParentFile.exists()) {
assertTrue("Failed to create parent (" + ruleBaseParentFile + ") for " +
@@ -346,6 +405,9 @@
@Override
protected void configure() {
+ bind(String.class).annotatedWith(PluginName.class)
+ .toInstance("ItsTestName");
+
sitePath = randomTargetFile();
assertFalse("sitePath already (" + sitePath + ") already exists",
sitePath.exists());
