RuleBase: Simplify reading rules files
* Use Path to resolve the 'etc/its' folder instead of using strings and
file separators.
* Simplify the configuration file name pattern.
* In the tests, give to the Enum defining the types of config files the
task of building the config file name and avoid switching over its
values.
Given the path normalization, we no longer need to get the canonical
path when warning about not configuration files being present, which
simplifies further the code.
Change-Id: I5bddb23af6caf38bcd791cec0e870b27827cc68a
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBase.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBase.java
index ee0c568..748f4f6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBase.java
@@ -16,7 +16,7 @@
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.annotations.PluginName;
-import com.google.gerrit.server.config.SitePath;
+import com.google.gerrit.server.config.SitePaths;
import com.google.inject.Inject;
import java.io.File;
import java.io.IOException;
@@ -32,12 +32,8 @@
public class RuleBase {
private static final Logger log = LoggerFactory.getLogger(RuleBase.class);
- /** File beginning (relative to site) to load rules from */
- private static final String ITS_CONFIG_FILE_START =
- "etc" + File.separatorChar + "its" + File.separator + "actions";
-
- /** File end to load rules from */
- private static final String ITS_CONFIG_FILE_END = ".config";
+ /** Rules configuration filename pattern */
+ private static final String CONFIG_FILE_NAME = "actions%s.config";
/** The section for rules within rulebases */
private static final String RULE_SECTION = "rule";
@@ -45,7 +41,7 @@
/** The key for actions within rulebases */
private static final String ACTION_KEY = "action";
- private final Path sitePath;
+ private final Path itsPath;
private final Rule.Factory ruleFactory;
private final Condition.Factory conditionFactory;
private final ActionRequest.Factory actionRequestFactory;
@@ -59,12 +55,12 @@
@Inject
public RuleBase(
- @SitePath Path sitePath,
+ SitePaths sitePaths,
Rule.Factory ruleFactory,
Condition.Factory conditionFactory,
ActionRequest.Factory actionRequestFactory,
@PluginName String pluginName) {
- this.sitePath = sitePath;
+ this.itsPath = sitePaths.etc_dir.normalize().resolve("its");
this.ruleFactory = ruleFactory;
this.conditionFactory = conditionFactory;
this.actionRequestFactory = actionRequestFactory;
@@ -117,24 +113,19 @@
rules = Lists.newArrayList();
// Add global rules
- File globalRuleFile = new File(sitePath.toFile(), ITS_CONFIG_FILE_START + ITS_CONFIG_FILE_END);
+ File globalRuleFile = itsPath.resolve(String.format(CONFIG_FILE_NAME, "")).toFile();
addRulesFromFile(globalRuleFile);
// Add its-specific rules
File itsSpecificRuleFile =
- new File(sitePath.toFile(), ITS_CONFIG_FILE_START + "-" + pluginName + ITS_CONFIG_FILE_END);
+ itsPath.resolve(String.format(CONFIG_FILE_NAME, "-" + pluginName)).toFile();
addRulesFromFile(itsSpecificRuleFile);
if (!globalRuleFile.exists() && !itsSpecificRuleFile.exists()) {
- try {
- log.warn(
- "Neither global rule file {} nor Its specific rule file {} exist. Please configure rules.",
- globalRuleFile.getCanonicalPath(),
- itsSpecificRuleFile.getCanonicalPath());
- } catch (IOException e) {
- log.warn(
- "Neither global rule file nor Its specific rule files exist. Please configure rules.");
- }
+ log.warn(
+ "Neither global rule file {} nor Its specific rule file {} exist. Please configure rules.",
+ globalRuleFile,
+ itsSpecificRuleFile);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBaseTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBaseTest.java
index 371b9d5..076ce0a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBaseTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBaseTest.java
@@ -22,9 +22,6 @@
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase;
-import java.io.BufferedWriter;
-import java.io.File;
-import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -46,8 +43,14 @@
private boolean cleanupSitePath;
private enum RuleBaseKind {
- GLOBAL,
- ITS,
+ GLOBAL("actions"),
+ ITS("actions-ItsTestName");
+
+ String fileName;
+
+ RuleBaseKind(String fileName) {
+ this.fileName = fileName + ".config";
+ }
}
public void testWarnNonExistingRuleBase() {
@@ -291,32 +294,9 @@
}
private void injectRuleBase(String rules, RuleBaseKind ruleBaseKind) throws IOException {
- String baseName = "";
- switch (ruleBaseKind) {
- case GLOBAL:
- baseName = "actions";
- break;
- case ITS:
- baseName = "actions-ItsTestName";
- break;
- default:
- fail("Unknown ruleBaseKind");
- }
- File ruleBaseFile =
- new File(
- sitePath.toFile(),
- "etc" + File.separatorChar + "its" + File.separator + baseName + ".config");
-
- File ruleBaseParentFile = ruleBaseFile.getParentFile();
- if (!ruleBaseParentFile.exists()) {
- assertTrue(
- "Failed to create parent (" + ruleBaseParentFile + ") for " + "rule base",
- ruleBaseParentFile.mkdirs());
- }
- try (FileWriter unbufferedWriter = new FileWriter(ruleBaseFile);
- BufferedWriter writer = new BufferedWriter(unbufferedWriter)) {
- writer.write(rules);
- }
+ Path ruleBaseFile = sitePath.resolve("etc").resolve("its").resolve(ruleBaseKind.fileName);
+ Files.createDirectories(ruleBaseFile.getParent());
+ Files.write(ruleBaseFile, rules.getBytes());
}
@Override
