Add ownersFileName and test, clean up doc and code.
* Set ownersFileName to change default name OWNERS for a project or globally.
* Update config.md and fix an error description about addDebugMsg.
* Group Config variables and constants in Config.java.
Change-Id: I692e0d15831fb0a8872cb4f6110a44cf100d3adf
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
index 7d4aa0c..48f86e6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
@@ -17,6 +17,7 @@
import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.cache.CacheBuilder;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import java.util.Collection;
@@ -90,17 +91,23 @@
/** Returns a cached or new OwnersDb, for the specified patchset. */
OwnersDb get(Repository repository, ChangeData changeData, int patchset) throws OrmException {
+ Project.NameKey project = changeData.change().getProject();
String branch = changeData.change().getDest().get();
String dbKey = Cache.makeKey(changeData.getId().get(), patchset, branch);
// TODO: get changed files of the given patchset?
- return get(dbKey, repository, branch, changeData.currentFilePaths());
+ return get(dbKey, repository, project, branch, changeData.currentFilePaths());
}
/** Returns a cached or new OwnersDb, for the specified branch and changed files. */
- OwnersDb get(String key, Repository repository, String branch, Collection<String> files) {
+ OwnersDb get(
+ String key,
+ Repository repository,
+ Project.NameKey project,
+ String branch,
+ Collection<String> files) {
if (dbCache == null) { // Do not cache OwnersDb
log.trace("Create new OwnersDb, key=" + key);
- return new OwnersDb(key, repository, branch, files);
+ return new OwnersDb(key, repository, project, branch, files);
}
try {
log.trace("Get from cash " + dbCache + ", key=" + key + ", cache size=" + dbCache.size());
@@ -110,12 +117,12 @@
@Override
public OwnersDb call() {
log.trace("Create new OwnersDb, key=" + key);
- return new OwnersDb(key, repository, branch, files);
+ return new OwnersDb(key, repository, project, branch, files);
}
});
} catch (ExecutionException e) {
log.error("Cache.get has exception: " + e);
- return new OwnersDb(key, repository, branch, files);
+ return new OwnersDb(key, repository, project, branch, files);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
index cdf9d56..d7008b3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
@@ -26,17 +26,22 @@
/** find-owners configuration parameters */
class Config {
- // Name of config parameters and plugin.
- static final String SECTION = "findowners"; // used in Plugin config file
+ // Name of config parameters.
static final String ADD_DEBUG_MSG = "addDebugMsg";
- static final String MIN_OWNER_VOTE_LEVEL = "minOwnerVoteLevel";
+ static final String ALWAYS_SHOW_BUTTON = "alwaysShowButton"; // always show "Find Owners" button
static final String MAX_CACHE_AGE = "maxCacheAge"; // seconds to stay in cache
static final String MAX_CACHE_SIZE = "maxCacheSize"; // number of OwnersDb in cache
+ static final String MIN_OWNER_VOTE_LEVEL = "minOwnerVoteLevel"; // default +1
+ static final String OWNERS_FILE_NAME = "ownersFileName"; // default "OWNERS"
static final String REPORT_SYNTAX_ERROR = "reportSyntaxError";
- static final String ALWAYS_SHOW_BUTTON = "alwaysShowButton"; // always show "Find Owners" button
+
+ // Name of plugin and namespace.
static final String PLUGIN_NAME = "find-owners";
static final String PROLOG_NAMESPACE = "find_owners";
+ // Default values.
+ private static final String DEFAULT_OWNERS_FILE_NAME = "OWNERS";
+
// Global/plugin config parameters.
private static PluginConfigFactory config = null;
private static boolean addDebugMsg = false;
@@ -83,6 +88,26 @@
return alwaysShowButton;
}
+ static String getOwnersFileName(Project.NameKey project) {
+ if (config != null && project != null) {
+ try {
+ String name =
+ config
+ .getFromProjectConfigWithInheritance(project, PLUGIN_NAME)
+ .getString(OWNERS_FILE_NAME, DEFAULT_OWNERS_FILE_NAME);
+ if (name.trim().equals("")) {
+ log.error(
+ "Project " + project.get() + " has wrong " + OWNERS_FILE_NAME + ": \"" + name + "\"");
+ return DEFAULT_OWNERS_FILE_NAME;
+ }
+ return name;
+ } catch (NoSuchProjectException e) {
+ log.error("Cannot find project: " + project);
+ }
+ }
+ return DEFAULT_OWNERS_FILE_NAME;
+ }
+
@VisibleForTesting
static void setReportSyntaxError(boolean value) {
reportSyntaxError = value;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
index 04a087a..b2ce1f9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
@@ -16,6 +16,7 @@
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.gerrit.reviewdb.client.Project;
import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
@@ -51,8 +52,14 @@
OwnersDb() {}
- OwnersDb(String key, Repository repository, String branch, Collection<String> files) {
+ OwnersDb(
+ String key,
+ Repository repository,
+ Project.NameKey project,
+ String branch,
+ Collection<String> files) {
this.key = key;
+ String ownersFileName = Config.getOwnersFileName(project);
for (String fileName : files) {
// Find OWNERS in fileName's directory and parent directories.
// Stop looking for a parent directory if OWNERS has "set noparent".
@@ -60,10 +67,10 @@
String dir = Util.normalizedDirPath(fileName); // e.g. dir = ./d1/d2
while (!readDirs.contains(dir)) {
readDirs.add(dir);
- String filePath = (dir + "/OWNERS").substring(2); // remove "./"
+ String filePath = (dir + "/" + ownersFileName).substring(2); // remove "./"
String content = getRepositoryFile(repository, branch, filePath);
if (content != null && !content.equals("")) {
- addFile(dir + "/", dir + "/OWNERS", content.split("\\R+"));
+ addFile(dir + "/", dir + "/" + ownersFileName, content.split("\\R+"));
}
if (stopLooking.contains(dir + "/") || !dir.contains("/")) {
break; // stop looking through parent directory
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 91dfea0..d754912 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -61,6 +61,12 @@
are applied, the default requirement is **+1** Code-Review
vote from at least one owner of every changed file.
+## Default OWNERS file name
+
+This plugin finds owners in default OWNERS files.
+If a project has already used OWNERS files for other purpose,
+the "ownersFileName" parameter can be used to change the default.
+
## Example 0, call `submit_filter/2`
The simplest configuration adds to `rules.pl` of the root
@@ -135,11 +141,22 @@
## Example 6, define `addDebugMsg`
-Add the following to global `gerrit.config`
-or a project's `project.config` file,
+Add the following to global `gerrit.config`,
to debug the find-owners plugin.
```bash
[plugin "find-owners"]
addDebugMsg = true
```
+
+## Example 7, change default OWNERS file name
+
+Add the following to global `gerrit.config`
+or a project's `project.config` file,
+to change the default "OWNERS" file name,
+e.g., to "OWNERS.android".
+
+```bash
+[plugin "find-owners"]
+ ownersFileName = OWNERS.android
+```
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
index 1501b3e..c8b3b3e 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
@@ -15,6 +15,8 @@
package com.googlesource.gerrit.plugins.findowners;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
+import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -26,13 +28,24 @@
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.inject.Inject;
+import org.eclipse.jgit.lib.ObjectLoader;
+import org.eclipse.jgit.revwalk.RevObject;
+import org.eclipse.jgit.revwalk.RevTree;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.RefSpec;
import org.junit.Test;
/** Test find-owners plugin API. */
@TestPlugin(name = "find-owners", sysModule = "com.googlesource.gerrit.plugins.findowners.Module")
public class FindOwnersIT extends LightweightPluginDaemonTest {
+ @Inject private PluginConfigFactory configFactory;
+
@Test
public void getOwnersTest() throws Exception {
ChangeInfo info1 = newChangeInfo("test1 GetOwners");
@@ -171,6 +184,71 @@
}
@Test
+ public void ownersFileNameTest() throws Exception {
+ Config.setVariables("find-owners", configFactory);
+
+ // Default project is something like ....FindOwnersIT..._project
+ Project.NameKey pA = createProject("Project_A");
+ Project.NameKey pB = createProject("Project_B");
+ switchProject(pA);
+ // Now project is ....FindOwnersIT..._Project_A
+ switchProject(pB);
+ // Now project is ....FindOwnersIT..._Project_B
+
+ // Add OWNERS and OWNERS.alpha file to Project_A.
+ switchProject(pA);
+ addFile("add OWNERS", "OWNERS", "per-file *.c=x@x\n"); // default owner x@x
+ addFile("add OWNERS.alpha", "OWNERS.alpha", "per-file *.c=a@a\n"); // alpha owner a@a
+ PushOneCommit.Result cA = createChange("add tA.c", "tA.c", "Hello A!");
+
+ // Add OWNERS and OWNERS.beta file to Project_B.
+ switchProject(pB);
+ addFile("add OWNERS", "OWNERS", "per-file *.c=y@y\n"); // default owner y@y
+ addFile("add OWNERS.beta", "OWNERS.beta", "per-file *.c=b@b\n"); // beta owner b@b
+ PushOneCommit.Result cB = createChange("add tB.c", "tB.c", "Hello B!");
+
+ // Default owners file name is "OWNERS".
+ assertThat(Config.getOwnersFileName(null)).isEqualTo("OWNERS");
+ assertThat(Config.getOwnersFileName(pA)).isEqualTo("OWNERS");
+ assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS");
+
+ assertThat(getOwnersResponse(cA)).contains("owners:[ x@x[1+0+0] ], files:[ tA.c ]");
+ assertThat(getOwnersResponse(cB)).contains("owners:[ y@y[1+0+0] ], files:[ tB.c ]");
+
+ // Change owners file name to "OWNERS.alpha" and "OWNERS.beta"
+ switchProject(pA);
+ setProjectConfig("ownersFileName", "OWNERS.alpha");
+ switchProject(pB);
+ setProjectConfig("ownersFileName", "OWNERS.beta");
+ assertThat(Config.getOwnersFileName(pA)).isEqualTo("OWNERS.alpha");
+ assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS.beta");
+ assertThat(getOwnersResponse(cA)).contains("owners:[ a@a[1+0+0] ], files:[ tA.c ]");
+ assertThat(getOwnersResponse(cB)).contains("owners:[ b@b[1+0+0] ], files:[ tB.c ]");
+
+ // Change back to OWNERS in Project_A
+ switchProject(pA);
+ setProjectConfig("ownersFileName", "OWNERS");
+ assertThat(Config.getOwnersFileName(pA)).isEqualTo("OWNERS");
+ assertThat(getOwnersResponse(cA)).contains("owners:[ x@x[1+0+0] ], files:[ tA.c ]");
+ assertThat(getOwnersResponse(cB)).contains("owners:[ b@b[1+0+0] ], files:[ tB.c ]");
+
+ // Change back to OWNERS.alpha in Project_B, but there is no OWNERS.alpha
+ switchProject(pB);
+ setProjectConfig("ownersFileName", "OWNERS.alpha");
+ assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS.alpha");
+ assertThat(getOwnersResponse(cA)).contains("owners:[ x@x[1+0+0] ], files:[ tA.c ]");
+ assertThat(getOwnersResponse(cB)).contains("owners:[], files:[ tB.c ]");
+
+ // Do not accept empty string or all-white-spaces for ownersFileName.
+ setProjectConfig("ownersFileName", " ");
+ assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS");
+ setProjectConfig("ownersFileName", " \t ");
+ assertThat(Config.getOwnersFileName(pB)).isEqualTo("OWNERS");
+ setProjectConfig("ownersFileName", "O");
+ assertThat(Config.getOwnersFileName(pB)).isEqualTo("O");
+ }
+
+ @Test
public void actionApplyTest() throws Exception {
Cache cache = Cache.getInstance().init(0, 10);
assertThat(cache.size()).isEqualTo(0);
@@ -239,6 +317,42 @@
approveSubmit(createChange(subject, file, content));
}
+ private void switchProject(Project.NameKey p) throws Exception {
+ project = p;
+ testRepo = cloneProject(project);
+ }
+
+ private org.eclipse.jgit.lib.Config readProjectConfig() throws Exception {
+ git().fetch().setRefSpecs(new RefSpec(REFS_CONFIG + ":" + REFS_CONFIG)).call();
+ testRepo.reset(RefNames.REFS_CONFIG);
+ RevWalk rw = testRepo.getRevWalk();
+ RevTree tree = rw.parseTree(testRepo.getRepository().resolve("HEAD"));
+ RevObject obj = rw.parseAny(testRepo.get(tree, "project.config"));
+ ObjectLoader loader = rw.getObjectReader().open(obj);
+ String text = new String(loader.getCachedBytes(), UTF_8);
+ org.eclipse.jgit.lib.Config cfg = new org.eclipse.jgit.lib.Config();
+ cfg.fromText(text);
+ return cfg;
+ }
+
+ private void setProjectConfig(String var, String value) throws Exception {
+ org.eclipse.jgit.lib.Config cfg = readProjectConfig();
+ cfg.setString("plugin", "find-owners", var, value);
+ assertThat(cfg.getString("plugin", "find-owners", var)).isEqualTo(value);
+ PushOneCommit.Result commit =
+ pushFactory
+ .create(
+ db,
+ admin.getIdent(), // normal user cannot change refs/meta/config
+ testRepo,
+ "Update project config",
+ "project.config",
+ cfg.toText())
+ .to("refs/for/" + REFS_CONFIG);
+ commit.assertOkStatus();
+ approveSubmit(commit);
+ }
+
// Remove '"' and space; replace '\n' with ' '; ignore unpredictable "owner_revision".
private static String filteredJson(String json) {
return json.replaceAll("[\" ]*", "").replace('\n', ' ').replaceAll("owner_revision:[^ ]* ", "");