Do not use HTTP requests, add integration tests, etc.
These changes are required to match Google Java coding style
and run on Google Gerrit production servers.
* Replace old calls to Gerrit REST API and Json processing
with calls to Gerrit Java APIs.
* Remove mocked classes in unit tests; replace them with integration test.
* Use Google Java Coding Style and google-java-format,
with line length limit of 100 characters.
* Replace Servlet with RestReadView<ChangeResource>
* Delegate GetOwners.apply to Action.apply.
* Update rest-api.md; move path2owners and owner2paths
into dbgmsgs of REST API result.
* Remove the "server" url in returned JSON debug info.
* Fix "owner_revision" bug in returned JSON.
* Make the Cache configurable through maxCacheAge and maxCacheSize,
with default no Cache and com.google.common.cache.Cache.
* Throw exceptions instead of returning error code;
use BadRequestException.
* Remove TRACE_SERVER_MSG, reduce some debug/trace messages.
* Decouple the Parser and OwnersDb classes.
* Do without String2*, StringSet, Owner2Weights classes.
* Replace Action.Input with Action.Parameters.
* Read config parameters from find-owners.config.
* Remove Server, move methods into Action, Checker, OwnersDb.
* Use AccountCache.
Change-Id: I502e29018247c389fa9ffffd3e5fce1815a4cd08
diff --git a/BUILD b/BUILD
index 7244080..c567c29 100644
--- a/BUILD
+++ b/BUILD
@@ -24,7 +24,6 @@
manifest_entries = [
'Gerrit-PluginName: find-owners',
'Gerrit-ReloadMode: restart',
- 'Gerrit-HttpModule: com.googlesource.gerrit.plugins.findowners.Servlet',
'Gerrit-Module: com.googlesource.gerrit.plugins.findowners.Module',
'Implementation-Title: Find-Owners plugin',
'Implementation-URL: https://gerrit.googlesource.com/plugins/find-owners',
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
index 48fd7ca..f5972d6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Action.java
@@ -14,75 +14,83 @@
package com.googlesource.gerrit.plugins.findowners;
-import com.google.gerrit.extensions.annotations.PluginCanonicalWebUrl;
-import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.webui.UiAction;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
+import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
-import com.google.gson.JsonArray;
-import com.google.gson.JsonObject;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import com.googlesource.gerrit.plugins.findowners.Util.Owner2Weights;
-import com.googlesource.gerrit.plugins.findowners.Util.String2String;
-import com.googlesource.gerrit.plugins.findowners.Util.String2StringSet;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
+import java.io.IOException;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
+import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** Create and return OWNERS info when "Find Owners" button is clicked. */
-class Action implements UiAction<RevisionResource>,
- RestModifyView<RevisionResource, Action.Input> {
+class Action implements RestReadView<RevisionResource>, UiAction<RevisionResource> {
private static final Logger log = LoggerFactory.getLogger(Action.class);
- private Provider<CurrentUser> user; // is null if from HTTP request
- private String url; // from REST request or client action call
- private Server server;
+ private AccountCache accountCache;
+ private ChangeData.Factory changeDataFactory;
+ private GitRepositoryManager repoManager;
+ private Provider<CurrentUser> userProvider;
+ private SchemaFactory<ReviewDb> reviewDbProvider;
- static class Input {
- // Only the change number is required.
- int change; // Revision change number
- String debug; // REST API parameter, 1, true/false, yes
- String patchset; // REST API parameter, patchset number
-
- Input(int change, String2String params) {
- this.change = change;
- debug = params.get("debug");
- patchset = params.get("patchset");
- // other keys in params are ignored
- }
+ static class Parameters {
+ Boolean debug; // REST API "debug" parameter, or null
+ Integer patchset; // REST API "patchset" parameter, or null
}
@Inject
- Action(@PluginCanonicalWebUrl String url, Provider<CurrentUser> user) {
- this.url = Util.normalizeURL(url); // replace "http:///" with "http://"
- int n = this.url.indexOf("/plugins/");
- if (n > 0) { // remove suffix "plugins/find-owners/...."
- this.url = this.url.substring(0, n + 1);
- }
- this.user = user;
- server = new Server();
+ Action(
+ Provider<CurrentUser> userProvider,
+ SchemaFactory<ReviewDb> reviewDbProvider,
+ ChangeData.Factory changeDataFactory,
+ AccountCache accountCache,
+ GitRepositoryManager repoManager) {
+ this.userProvider = userProvider;
+ this.reviewDbProvider = reviewDbProvider;
+ this.changeDataFactory = changeDataFactory;
+ this.accountCache = accountCache;
+ this.repoManager = repoManager;
}
- /** Used by unit tests to set up mocked Server. */
- void setServer(Server s) {
- server = s;
- }
+ @VisibleForTesting
+ Action() {}
private String getUserName() {
- return (null != user) ? user.get().getUserName() : "?";
+ if (userProvider != null && userProvider.get().getUserName() != null) {
+ return userProvider.get().getUserName();
+ }
+ return "?";
}
- private JsonArray getOwners(OwnersDb db, Collection<String> files) {
- Owner2Weights weights = new Owner2Weights();
- String2StringSet file2Owners = db.findOwners(files, weights);
- JsonArray result = new JsonArray();
- StringSet emails = new StringSet();
+ private List<String> getOwners(OwnersDb db, Collection<String> files) {
+ Map<String, OwnerWeights> weights = new HashMap<>();
+ db.findOwners(files, weights);
+ List<String> result = new ArrayList<>();
+ Set<String> emails = new HashSet<>();
for (String key : OwnerWeights.sortKeys(weights)) {
if (!emails.contains(key)) {
result.add(key + " " + weights.get(key).encodeLevelCounts());
@@ -92,83 +100,118 @@
return result;
}
- private void addNamedMap(JsonObject obj, String name,
- Map<String, StringSet> map) {
- JsonObject jsonMap = new JsonObject();
- for (String key : Util.sort(map.keySet())) {
- jsonMap.addProperty(key, String.join(" ", Util.sort(map.get(key))));
+ @Override
+ public Response<RestResult> apply(RevisionResource rev)
+ throws IOException, OrmException, BadRequestException {
+ return apply(rev.getChangeResource(), new Parameters());
+ }
+
+ // Used by both Action.apply and GetOwners.apply.
+ public Response<RestResult> apply(ChangeResource rsrc, Parameters params)
+ throws IOException, OrmException, BadRequestException {
+ try (ReviewDb reviewDb = reviewDbProvider.open()) {
+ return apply(reviewDb, rsrc, params);
}
- obj.add(name, jsonMap);
+ }
+
+ // Used by integration tests, because they do not have ReviewDb Provider.
+ public Response<RestResult> apply(ReviewDb reviewDb, ChangeResource rsrc, Parameters params)
+ throws IOException, OrmException, BadRequestException {
+ Change c = rsrc.getChange();
+ try (Repository repo = repoManager.openRepository(c.getProject())) {
+ ChangeData changeData = changeDataFactory.create(reviewDb, c);
+ return getChangeData(repo, params, changeData);
+ }
+ }
+
+ /** Returns reviewer emails got from ChangeData. */
+ static List<String> getReviewers(ChangeData changeData, AccountCache accountCache) {
+ List<String> result = new ArrayList<>();
+ try {
+ for (Account.Id id : changeData.reviewers().all()) {
+ Account account = accountCache.get(id).getAccount();
+ result.add(account.getPreferredEmail() + " []");
+ }
+ } catch (OrmException e) {
+ log.error("Exception", e);
+ result = new ArrayList<>();
+ }
+ return result;
+ }
+
+ /** Returns the current patchset number or the given patchsetNum if it is valid. */
+ static int getValidPatchsetNum(ChangeData changeData, Integer patchsetNum)
+ throws OrmException, BadRequestException {
+ int patchset = changeData.currentPatchSet().getId().get();
+ if (patchsetNum != null) {
+ if (patchsetNum < 1 || patchsetNum > patchset) {
+ throw new BadRequestException(
+ "Invalid patchset parameter: "
+ + patchsetNum
+ + "; must be 1"
+ + ((1 != patchset) ? (" to " + patchset) : ""));
+ }
+ return patchsetNum;
+ }
+ return patchset;
}
/** REST API to return owners info of a change. */
- public JsonObject getChangeData(int change, String2String params) {
- return apply(null, new Input(change, params));
- }
+ public Response<RestResult> getChangeData(
+ Repository repository, Parameters params, ChangeData changeData)
+ throws OrmException, BadRequestException {
+ int patchset = getValidPatchsetNum(changeData, params.patchset);
+ OwnersDb db = Cache.getInstance().get(repository, changeData, patchset);
+ Collection<String> changedFiles = changeData.currentFilePaths();
+ Map<String, Set<String>> file2Owners = db.findOwners(changedFiles);
- /** Called by the client "Find Owners" button. */
- @Override
- public JsonObject apply(RevisionResource rev, Input input) {
- server.setChangeId(url, input.change);
- String error = (null != server.error)
- ? server.error : server.setPatchId(input.patchset);
- if (null != error) {
- JsonObject obj = new JsonObject();
- obj.addProperty("error", error);
- return obj;
- }
- OwnersDb db = server.getCachedOwnersDb();
- Collection<String> changedFiles = server.getChangedFiles();
- String2StringSet file2Owners = db.findOwners(changedFiles);
-
- JsonObject obj = new JsonObject();
- obj.addProperty(Config.MIN_OWNER_VOTE_LEVEL, server.getMinOwnerVoteLevel());
- boolean addDebugMsg = (null != input.debug)
- ? Util.parseBoolean(input.debug) : server.getAddDebugMsg();
- obj.addProperty(Config.ADD_DEBUG_MSG, addDebugMsg);
- obj.addProperty("change", input.change);
- obj.addProperty("patchset", server.patchset);
- obj.addProperty("owner_revision", db.revision);
-
+ Boolean addDebugMsg = (params.debug != null) ? params.debug : Config.getAddDebugMsg();
+ RestResult obj = new RestResult(Config.getMinOwnerVoteLevel(changeData), addDebugMsg);
+ obj.change = changeData.getId().get();
+ obj.patchset = patchset;
+ obj.ownerRevision = db.revision;
if (addDebugMsg) {
- JsonObject dbgMsgObj = new JsonObject();
- dbgMsgObj.addProperty("user", getUserName());
- dbgMsgObj.addProperty("project", server.project);
- dbgMsgObj.addProperty("branch", server.branch);
- dbgMsgObj.addProperty("server", url);
- obj.add("dbgmsgs", dbgMsgObj);
- addNamedMap(obj, "path2owners", db.path2Owners);
- addNamedMap(obj, "owner2paths", db.owner2Paths);
+ obj.dbgmsgs.user = getUserName();
+ obj.dbgmsgs.project = changeData.change().getProject().get();
+ obj.dbgmsgs.branch = changeData.change().getDest().get();
+ obj.dbgmsgs.path2owners = Util.makeSortedMap(db.path2Owners);
+ obj.dbgmsgs.owner2paths = Util.makeSortedMap(db.owner2Paths);
}
- addNamedMap(obj, "file2owners", file2Owners);
- obj.add("reviewers", server.getReviewers());
- obj.add("owners", getOwners(db, changedFiles));
- obj.add("files", Util.newJsonArrayFromStrings(changedFiles));
- return obj;
+ obj.file2owners = Util.makeSortedMap(file2Owners);
+ obj.reviewers = getReviewers(changeData, accountCache);
+ obj.owners = getOwners(db, changedFiles);
+ obj.files = new ArrayList<>(changedFiles);
+ return Response.ok(obj);
}
@Override
public Description getDescription(RevisionResource resource) {
- int change = resource.getChange().getId().get();
- server.setChangeId(url, change);
- if (null == server.branch) {
- log.error("Cannot get branch of change: " + change);
- return null; // no "Find Owners" button
+ Change change = resource.getChangeResource().getChange();
+ try (ReviewDb reviewDb = reviewDbProvider.open();
+ Repository repo = repoManager.openRepository(change.getProject())) {
+ ChangeData changeData = changeDataFactory.create(reviewDb, change);
+ if (changeData.change().getDest().get() == null) {
+ log.error("Cannot get branch of change: " + changeData.getId().get());
+ return null; // no "Find Owners" button
+ }
+ OwnersDb db = Cache.getInstance().get(repo, changeData);
+ log.trace("getDescription db key = " + db.key);
+ Status status = resource.getChange().getStatus();
+ // Commit message is not used to enable/disable "Find Owners".
+ boolean needFindOwners =
+ userProvider != null
+ && userProvider.get() instanceof IdentifiedUser
+ && (db.getNumOwners() > 0)
+ && status != Status.ABANDONED
+ && status != Status.MERGED;
+ return new Description()
+ .setLabel("Find Owners")
+ .setTitle("Find owners to add to Reviewers list")
+ .setVisible(needFindOwners);
+ } catch (IOException | OrmException e) {
+ log.error("Exception", e);
+ throw new IllegalStateException(e);
}
- OwnersDb db = server.getCachedOwnersDb();
- if (server.traceServerMsg()) {
- log.info(server.genDebugMsg(db));
- }
- Status status = server.getStatus(resource);
- // Commit message is not used to enable/disable "Find Owners".
- boolean needFindOwners =
- (null != user && user.get() instanceof IdentifiedUser)
- && (db.getNumOwners() > 0)
- && (status != Status.ABANDONED && status != Status.MERGED);
- return new Description()
- .setLabel("Find Owners")
- .setTitle("Find owners to add to Reviewers list")
- .setVisible(needFindOwners);
}
}
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 b92c790..7d4aa0c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Cache.java
@@ -14,15 +14,22 @@
package com.googlesource.gerrit.plugins.findowners;
+import static java.util.concurrent.TimeUnit.SECONDS;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
import java.util.Collection;
-import java.util.Date;
-import java.util.Deque;
-import java.util.HashMap;
-import java.util.LinkedList;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.lib.Repository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/** Save OwnersDb in a cache for multiple calls to submit_filter. */
class Cache {
+ private static final Logger log = LoggerFactory.getLogger(Cache.class);
+
// The OwnersDb is created from OWNERS files in directories that
// contain changed files of a patch set, which belongs to a project
// and branch. OwnersDb can be cached if the head of a project branch
@@ -37,90 +44,92 @@
// Each submit_filter has one Prolog engine.
// It would not be enough to keep the cache in a Prolog engine environment
// or a StoredValues.
- // We keep the cache in a Java class static object for all HTTP requests.
- // OwnersDb is cached for up to 10 seconds.
- private static final int CACHE_LIFE_MSEC = 10000;
+ private static Cache instance = null; // a singleton
// When branch is "refs/heads/xyz" use only "xyz",
// to share cached OwnersDb between these two branch names.
private static final String REFS_HEADS = "refs/heads/";
- static class CachedObj {
- long time; // system time in milliseconds, when db is created
- String key; // (changeId, patchSetId, branchName)
- OwnersDb db;
- CachedObj(String k, OwnersDb x) {
- time = new Date().getTime();
- key = k;
- db = x;
+ // Until we have a way to inject Java singleton into Prolog PRED_* constructors,
+ // we keep the cache in a static object for all HTTP requests.
+ private com.google.common.cache.Cache<String, OwnersDb> dbCache;
+
+ private Cache() {
+ init(Config.getMaxCacheAge(), Config.getMaxCacheSize());
+ }
+
+ long size() {
+ return (dbCache == null) ? 0 : dbCache.size();
+ }
+
+ Cache init(int maxSeconds, int maxSize) {
+ // This should be called once in normal configuration,
+ // but could be called multiple times in unit or integration tests.
+ if (dbCache != null) {
+ dbCache.invalidateAll(); // release all cached objects
+ }
+ if (maxSeconds > 0) {
+ log.info("Initialize Cache with maxSeconds=" + maxSeconds + " maxSize=" + maxSize);
+ dbCache =
+ CacheBuilder.newBuilder()
+ .maximumSize(maxSize)
+ .expireAfterWrite(maxSeconds, SECONDS)
+ .build();
+ } else {
+ log.info("Cache disabled.");
+ dbCache = null;
+ }
+ return this;
+ }
+
+ /** Returns a cached or new OwnersDb, for the current patchset. */
+ OwnersDb get(Repository repo, ChangeData changeData) throws OrmException {
+ return get(repo, changeData, changeData.currentPatchSet().getId().get());
+ }
+
+ /** Returns a cached or new OwnersDb, for the specified patchset. */
+ OwnersDb get(Repository repository, ChangeData changeData, int patchset) throws OrmException {
+ 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());
+ }
+
+ /** Returns a cached or new OwnersDb, for the specified branch and changed files. */
+ OwnersDb get(String key, Repository repository, 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);
+ }
+ try {
+ log.trace("Get from cash " + dbCache + ", key=" + key + ", cache size=" + dbCache.size());
+ return dbCache.get(
+ key,
+ new Callable<OwnersDb>() {
+ @Override
+ public OwnersDb call() {
+ log.trace("Create new OwnersDb, key=" + key);
+ return new OwnersDb(key, repository, branch, files);
+ }
+ });
+ } catch (ExecutionException e) {
+ log.error("Cache.get has exception: " + e);
+ return new OwnersDb(key, repository, branch, files);
}
}
- // Before a new CachedObj is added to the tail of dbQueue,
- // old and obsolete CachedObj are removed from the head.
- private static final Deque<CachedObj> dbQueue = new LinkedList<CachedObj>();
-
- // A HashMap provides quick lookup with a key.
- private static final HashMap<String, CachedObj> dbCache =
- new HashMap<String, CachedObj>();
-
- private static long minCachedObjectTime() {
- // Cached objects must be used within CACHE_LIFE_MSEC.
- return new Date().getTime() - CACHE_LIFE_MSEC;
- }
-
- static String makeKey(int change, int patch, String branch) {
+ public static String makeKey(int change, int patch, String branch) {
if (branch.indexOf(REFS_HEADS) == 0) {
branch = branch.substring(REFS_HEADS.length());
}
return change + ":" + patch + ":" + branch;
}
- private static void saveCachedDb(String key, OwnersDb db) {
- CachedObj obj = new CachedObj(key, db);
- long minTime = minCachedObjectTime();
- synchronized (dbCache) {
- // Remove cached objects older than minTime.
- while (dbQueue.size() > 0 && dbQueue.peek().time < minTime) {
- dbCache.remove(dbQueue.peek().key);
- dbQueue.removeFirst();
- }
- // Add the new one to the tail.
- dbCache.put(key, obj);
- dbQueue.addLast(obj);
+ public static Cache getInstance() {
+ if (instance == null) {
+ instance = new Cache();
}
- }
-
- static OwnersDb get(Server server, String key, String url, String project,
- String branch, Collection<String> files) {
- return get(server, key, null, url, project, branch, files);
- }
-
- static OwnersDb get(Server server, String key, Repository repository,
- String branch, Collection<String> files) {
- return get(server, key, repository, null, null, branch, files);
- }
-
- private static OwnersDb get(
- Server server, String key, Repository repository, String url,
- String project, String branch, Collection<String> files) {
- OwnersDb db = null;
- long minTime = minCachedObjectTime();
- synchronized (dbCache) {
- if (dbCache.containsKey(key)) {
- CachedObj obj = dbCache.get(key);
- if (obj.time >= minTime) {
- db = obj.db;
- }
- }
- }
- if (null == db) {
- db = (null != repository)
- ? new OwnersDb(server, key, repository, branch, files)
- : new OwnersDb(server, key, url, project, branch, files);
- saveCachedDb(key, db);
- }
- return db;
+ return instance;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
index 4477e17..8cc92ac 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Checker.java
@@ -14,10 +14,20 @@
package com.googlesource.gerrit.plugins.findowners;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change.Status;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.server.AccountAccess;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.rules.StoredValues;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
import com.googlecode.prolog_cafe.lang.Prolog;
-import com.googlesource.gerrit.plugins.findowners.Util.String2Integer;
-import com.googlesource.gerrit.plugins.findowners.Util.String2StringSet;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -25,16 +35,40 @@
public class Checker {
private static final Logger log = LoggerFactory.getLogger(Checker.class);
- private Server server;
+ // Accept both "Exempt-" and "Exempted-".
+ private static final String EXEMPT_MESSAGE1 = "Exempt-From-Owner-Approval:";
+ private static final String EXEMPT_MESSAGE2 = "Exempted-From-Owner-Approval:";
+
+ private Repository repository;
+ private ChangeData changeData;
private int minVoteLevel;
- Checker(Server s, int v) {
+ Checker(Repository repository, ChangeData changeData, int v) {
+ this.repository = repository;
+ this.changeData = changeData;
minVoteLevel = v;
- server = s; // could be a mocked server
+ }
+
+ /** Returns a map from reviewer email to vote value. */
+ static Map<String, Integer> getVotes(ChangeData changeData) throws OrmException {
+ ReviewDb db = changeData.db();
+ Map<String, Integer> map = new HashMap<>();
+ AccountAccess ac = db.accounts();
+ for (PatchSetApproval p : changeData.currentApprovals()) {
+ if (p.getValue() != 0) {
+ Account.Id id = p.getAccountId();
+ try {
+ map.put(ac.get(id).getPreferredEmail(), Integer.valueOf(p.getValue()));
+ } catch (OrmException e) {
+ log.error("Cannot get email address of account id: " + id.get(), e);
+ }
+ }
+ }
+ return map;
}
/** Returns true if some owner in owners is "*" or in votes */
- boolean findOwnersInVotes(StringSet owners, String2Integer votes) {
+ boolean findOwnersInVotes(Set<String> owners, Map<String, Integer> votes) {
boolean foundVeto = false;
boolean foundApproval = false;
for (String owner : owners) {
@@ -45,20 +79,20 @@
foundApproval |= (v >= minVoteLevel);
foundVeto |= (v < 0); // an owner's -1 vote is a veto
} else if (owner.equals("*")) {
- foundApproval = true; // no specific owner
+ foundApproval = true; // no specific owner
}
}
return foundApproval && !foundVeto;
}
/** Returns 1 if owner approval is found, -1 if missing, 0 if unneeded. */
- int findApproval(OwnersDb db) {
- String2StringSet file2Owners = db.findOwners(server.getChangedFiles());
- if (file2Owners.size() == 0) { // do not need owner approval
+ int findApproval(OwnersDb db) throws OrmException {
+ Map<String, Set<String>> file2Owners = db.findOwners(changeData.currentFilePaths());
+ if (file2Owners.size() == 0) { // do not need owner approval
return 0;
}
- String2Integer votes = server.getVotes();
- for (StringSet owners : file2Owners.values()) {
+ Map<String, Integer> votes = getVotes(changeData);
+ for (Set<String> owners : file2Owners.values()) {
if (!findOwnersInVotes(owners, votes)) {
return -1;
}
@@ -68,25 +102,46 @@
/** Returns 1 if owner approval is found, -1 if missing, 0 if unneeded. */
public static int findApproval(Prolog engine, int minVoteLevel) {
- return new Checker(new Server(engine), minVoteLevel).findApproval();
+ try {
+ ChangeData changeData = StoredValues.CHANGE_DATA.get(engine);
+ Repository repository = StoredValues.REPOSITORY.get(engine);
+ return new Checker(repository, changeData, minVoteLevel).findApproval();
+ } catch (OrmException e) {
+ log.error("Exception", e);
+ return 0; // owner approval may or may not be required.
+ }
}
- int findApproval() {
- if (server.isExemptFromOwnerApproval()) {
+ /** Returns true if exempt from owner approval. */
+ static boolean isExemptFromOwnerApproval(ChangeData changeData) throws OrmException {
+ try {
+ String message = changeData.commitMessage();
+ if (message.contains(EXEMPT_MESSAGE1) || message.contains(EXEMPT_MESSAGE2)) {
+ return true;
+ }
+ } catch (IOException | OrmException e) {
+ log.error("Cannot get commit message", e);
+ return true; // exempt from owner approval due to lack of data
+ }
+ // Abandoned and merged changes do not need approval again.
+ Status status = changeData.change().getStatus();
+ return (status == Status.ABANDONED || status == Status.MERGED);
+ }
+
+ int findApproval() throws OrmException {
+ if (isExemptFromOwnerApproval(changeData)) {
return 0;
}
// One update to a Gerrit change can call submit_rule or submit_filter
// many times. So this function should use cached values.
- OwnersDb db = server.getCachedOwnersDb();
+ OwnersDb db = Cache.getInstance().get(repository, changeData);
if (db.getNumOwners() <= 0) {
return 0;
}
if (minVoteLevel <= 0) {
- minVoteLevel = server.getMinOwnerVoteLevel();
+ minVoteLevel = Config.getMinOwnerVoteLevel(changeData);
}
- if (server.traceServerMsg()) {
- log.info(server.genDebugMsg(db));
- }
+ log.trace("findApproval db key = " + db.key);
return findApproval(db);
}
}
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 f6bb5bf..66c394c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Config.java
@@ -14,64 +14,82 @@
package com.googlesource.gerrit.plugins.findowners;
+import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** find-owners configuration parameters */
class Config {
// Name of config parameters and plugin.
+ static final String SECTION = "findowners"; // used in Plugin config file
static final String ADD_DEBUG_MSG = "addDebugMsg";
static final String MIN_OWNER_VOTE_LEVEL = "minOwnerVoteLevel";
+ 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 REPORT_SYNTAX_ERROR = "reportSyntaxError";
static final String PLUGIN_NAME = "find-owners";
static final String PROLOG_NAMESPACE = "find_owners";
- // Enable TRACE_SERVER_MSG only for dev/test builds.
- private static final boolean TRACE_SERVER_MSG = false;
-
// Global/plugin config parameters.
private static PluginConfigFactory config = null;
private static boolean addDebugMsg = false;
private static int minOwnerVoteLevel = 1;
+ private static int maxCacheAge = 0;
+ private static int maxCacheSize = 1000;
private static boolean reportSyntaxError = false;
private static final Logger log = LoggerFactory.getLogger(Config.class);
- static void setVariables(PluginConfigFactory conf,
- boolean dbgMsg, int voteLevel, boolean reportError) {
- if (TRACE_SERVER_MSG) {
- log.info("Set config parameters "
- + ADD_DEBUG_MSG + "=" + dbgMsg
- + ", " + MIN_OWNER_VOTE_LEVEL + "=" + voteLevel
- + ", " + REPORT_SYNTAX_ERROR + "=" + reportError);
- }
+ static void setVariables(
+ PluginConfigFactory conf, PluginConfig gc, org.eclipse.jgit.lib.Config pc) {
+ // Get config variables from pc, or from gc.
config = conf;
- addDebugMsg = dbgMsg;
- minOwnerVoteLevel = voteLevel;
- reportSyntaxError = reportError;
- }
-
- static boolean traceServerMsg() {
- return TRACE_SERVER_MSG && addDebugMsg;
+ addDebugMsg =
+ pc.getBoolean(SECTION, null, ADD_DEBUG_MSG, gc.getBoolean(Config.ADD_DEBUG_MSG, false));
+ reportSyntaxError =
+ pc.getBoolean(
+ SECTION, null, REPORT_SYNTAX_ERROR, gc.getBoolean(REPORT_SYNTAX_ERROR, false));
+ minOwnerVoteLevel =
+ pc.getInt(SECTION, null, MIN_OWNER_VOTE_LEVEL, gc.getInt(MIN_OWNER_VOTE_LEVEL, 1));
+ maxCacheAge = pc.getInt(SECTION, null, MAX_CACHE_AGE, gc.getInt(MAX_CACHE_AGE, 0));
+ maxCacheSize = pc.getInt(SECTION, null, MAX_CACHE_SIZE, gc.getInt(MAX_CACHE_SIZE, 1000));
}
static boolean getAddDebugMsg() {
return addDebugMsg; // defined globally, not per-project
}
+ static int getMaxCacheAge() {
+ return maxCacheAge;
+ }
+
+ static int getMaxCacheSize() {
+ return maxCacheSize;
+ }
+
static boolean getReportSyntaxError() {
return reportSyntaxError;
}
- static int getMinOwnerVoteLevel(Project.NameKey project) {
+ @VisibleForTesting
+ static void setReportSyntaxError(boolean value) {
+ reportSyntaxError = value;
+ }
+
+ static int getMinOwnerVoteLevel(ChangeData changeData) throws OrmException {
+ Project.NameKey project = changeData.change().getProject();
try {
- return (null == config || null == project) ? minOwnerVoteLevel
- : config.getFromProjectConfigWithInheritance(
- project, PLUGIN_NAME).getInt(MIN_OWNER_VOTE_LEVEL,
- minOwnerVoteLevel);
+ return (config == null || project == null)
+ ? minOwnerVoteLevel
+ : config
+ .getFromProjectConfigWithInheritance(project, PLUGIN_NAME)
+ .getInt(MIN_OWNER_VOTE_LEVEL, minOwnerVoteLevel);
} catch (NoSuchProjectException e) {
log.error("Cannot find project: " + project);
return minOwnerVoteLevel;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/GetChange.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/GetChange.java
deleted file mode 100644
index 244fd22..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/GetChange.java
+++ /dev/null
@@ -1,116 +0,0 @@
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.findowners;
-
-import com.google.gerrit.extensions.annotations.Export;
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
-import com.google.gson.JsonObject;
-import com.google.inject.Singleton;
-import com.googlesource.gerrit.plugins.findowners.Util.String2String;
-import java.io.IOException;
-import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/** Serves the HTTP GET /change/id request. */
-@Export("/change/*")
-@Singleton
-class GetChange extends HttpServlet {
- private static final Logger log = LoggerFactory.getLogger(GetChange.class);
-
- private static final String MAGIC_PREFIX = ")]}'"; // Gerrit REST specific
-
- private int getChangeId(String url) {
- // Expect url=".../plugins/find-owners/change/<digits>"
- final Pattern patChangeId =
- Pattern.compile("^.*/plugins/" + Config.PLUGIN_NAME
- + "/change/([0-9]+)/?$");
- Matcher m = patChangeId.matcher(url);
- return m.find() ? Integer.parseInt(m.group(1)) : 0;
- }
-
- private String2String parseParameters(HttpServletRequest req) {
- Map<String, String[]> map = req.getParameterMap();
- String2String params = new String2String();
- for (Map.Entry<String, String[]> entry : map.entrySet()) {
- String[] value = entry.getValue();
- // Use only the last definition, if there are multiple.
- params.put(entry.getKey(), value[value.length - 1]);
- }
- return params;
- }
-
- protected void doGet(HttpServletRequest req, HttpServletResponse res)
- throws IOException {
- String reqURL = req.getRequestURL().toString();
- // e.g. http://localhost:8082/plugins/find-owners/change/36
- String localAddress = req.getLocalAddr(); // e.g. 100.108.228.206
- int localPort = req.getLocalPort(); // e.g. 8080, not client port number
- String2String params = parseParameters(req);
- String url = "http://" + localAddress + ":" + localPort + "/";
- // TODO: recognize pp=0 parameter and Accept HTTP request header
- // to output compact JSON.
- int changeId = getChangeId(reqURL);
- if (Config.traceServerMsg()) {
- String paramsDump = "";
- for (Map.Entry<String, String> entry : params.entrySet()) {
- paramsDump += " " + entry.getKey() + "=" + entry.getValue();
- }
- log.info("goGet reqURL=" + reqURL
- + ", address=" + localAddress + ", port=" + localPort
- + ", changeId=" + changeId + ", params:" + paramsDump);
- }
- createResponse(url, params, changeId, res);
- }
-
- private void createErrorResponse(HttpServletResponse res,
- String msg) throws IOException {
- res.setContentType("text/plain");
- res.getWriter().println("Error: " + msg);
- }
-
- private void createResponse(
- String url, String2String params, int changeId,
- HttpServletResponse res) throws IOException {
- res.setCharacterEncoding("UTF-8");
- if (changeId > 0) {
- Action finder = new Action(url, null);
- JsonObject obj = finder.getChangeData(changeId, params);
- if (null != obj.get("error")) {
- createErrorResponse(res, obj.get("error").getAsString());
- } else {
- // Current prototype always returns pretty-printed JSON.
- // TODO: recognize HTTP Accept-Encoding request header "gzip",
- // to gzip compress the response.
- Gson gs = new GsonBuilder()
- .setPrettyPrinting().disableHtmlEscaping().create();
- res.setContentType("application/json");
- res.getWriter().println(MAGIC_PREFIX);
- res.getWriter().println(gs.toJson(obj));
- }
- } else {
- createErrorResponse(res,
- "Missing change number.\n"
- + "Usage: <baseURL>/plugins/"
- + Config.PLUGIN_NAME + "/change/<changeId>");
- }
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/GetOwners.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/GetOwners.java
new file mode 100644
index 0000000..bbf5619
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/GetOwners.java
@@ -0,0 +1,74 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.findowners;
+
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import org.kohsuke.args4j.Option;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** REST API to get owners of a change. */
+public class GetOwners implements RestReadView<ChangeResource> {
+ private static final Logger log = LoggerFactory.getLogger(GetOwners.class);
+
+ private Action action;
+
+ // "debug" could be true/yes/1 or false/no/0,
+ // when not specified configuration variable "addDebugMsg" is used.
+ @Option(name = "--debug", usage = "get extra debug info")
+ private String debug;
+
+ @Option(name = "--patchset", usage = "select change patchset number")
+ private Integer patchset;
+
+ @Inject
+ GetOwners(
+ Provider<CurrentUser> userProvider,
+ SchemaFactory<ReviewDb> reviewDbProvider,
+ ChangeData.Factory dataFactory,
+ AccountCache accountCache,
+ GitRepositoryManager repoManager) {
+ this.action =
+ new Action(userProvider, reviewDbProvider, dataFactory, accountCache, repoManager);
+ }
+
+ @Override
+ public Response<RestResult> apply(ChangeResource rsrc) throws IOException, OrmException {
+ Action.Parameters params = new Action.Parameters();
+ params.patchset = patchset;
+ params.debug = (debug != null) ? Util.parseBoolean(debug) : null;
+ try {
+ return this.action.apply(rsrc, params);
+ } catch (BadRequestException e) {
+ // Catch this exception to avoid too many call stack dumps
+ // from bad wrong client requests.
+ log.error("Exception: " + e);
+ return Response.none();
+ }
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Module.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Module.java
index 151ccba..13a3b5d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Module.java
@@ -14,25 +14,23 @@
package com.googlesource.gerrit.plugins.findowners;
+import static com.google.gerrit.server.change.ChangeResource.CHANGE_KIND;
import static com.google.gerrit.server.change.RevisionResource.REVISION_KIND;
import com.google.common.collect.ImmutableSet;
-import com.google.gerrit.extensions.annotations.Listen;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiModule;
import com.google.gerrit.extensions.webui.JavaScriptPlugin;
import com.google.gerrit.extensions.webui.WebUiPlugin;
import com.google.gerrit.rules.PredicateProvider;
-import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
/** find-owners plugin module */
public class Module extends AbstractModule {
- /** Prolog Predicate Provider. */
- @Listen
+ /** Prolog Predicate Provider. */
static class FindOwnersProvider implements PredicateProvider {
@Override
public ImmutableSet<String> getPackages() {
@@ -42,26 +40,25 @@
@Inject
public Module(@PluginName String pluginName, PluginConfigFactory config) {
- // The 'true' parameter does not seem to reread gerrit.config on restart.
- PluginConfig c = config.getFromGerritConfig(pluginName, true);
- // TODO: get config parameters from plugin config file.
- Config.setVariables(config,
- c.getBoolean(Config.ADD_DEBUG_MSG, false),
- c.getInt(Config.MIN_OWNER_VOTE_LEVEL, 1),
- c.getBoolean(Config.REPORT_SYNTAX_ERROR, false));
+ Config.setVariables(
+ config,
+ config.getFromGerritConfig(pluginName, true),
+ config.getGlobalPluginConfig(pluginName));
+ Cache.getInstance(); // Create a single Cache.
}
@Override
protected void configure() {
- install(new RestApiModule() {
- @Override
- protected void configure() {
- post(REVISION_KIND, Config.PLUGIN_NAME).to(Action.class);
- }
- });
+ install(
+ new RestApiModule() {
+ @Override
+ protected void configure() {
+ get(CHANGE_KIND, "owners").to(GetOwners.class);
+ get(REVISION_KIND, Config.PLUGIN_NAME).to(Action.class);
+ }
+ });
DynamicSet.bind(binder(), WebUiPlugin.class)
.toInstance(new JavaScriptPlugin(Config.PLUGIN_NAME + ".js"));
- DynamicSet.bind(binder(), PredicateProvider.class)
- .to(FindOwnersProvider.class);
+ DynamicSet.bind(binder(), PredicateProvider.class).to(FindOwnersProvider.class);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnerWeights.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnerWeights.java
index 5664349..d57304b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnerWeights.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnerWeights.java
@@ -14,33 +14,35 @@
package com.googlesource.gerrit.plugins.findowners;
-import com.googlesource.gerrit.plugins.findowners.Util.Owner2Weights;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
+import java.util.HashSet;
import java.util.List;
+import java.util.Map;
+import java.util.Set;
/**
* Keep owned files and count number of files at control level 1, 2, 3, etc.
*
- * <p>A source file can be owned by multiple OWNERS file in its directory or
- * parent directories. The owners listed in the lowest OWNERS file has
- * level 1 control of that source file. The 2nd lowest OWNERS file has
- * level 2 control, etc.
+ * <p>A source file can be owned by multiple OWNERS file in its directory or parent directories. The
+ * owners listed in the lowest OWNERS file has level 1 control of that source file. The 2nd lowest
+ * OWNERS file has level 2 control, etc.
+ *
* <p>An owner can own multiple source files at different control level.
- * <p>Each owner has an OwnerWeights object to keep
- * (0) the set of owned files,
- * (1) number of owned files with level 1 control,
- * (2) number of owned files with level 2 control,
- * (3) number of owned files with level 3 or higher control,
+ *
+ * <p>Each owner has an OwnerWeights object to keep (0) the set of owned files, (1) number of owned
+ * files with level 1 control, (2) number of owned files with level 2 control, (3) number of owned
+ * files with level 3 or higher control,
*/
class OwnerWeights {
static class WeightComparator implements Comparator<String> {
- private Owner2Weights map;
- WeightComparator(Owner2Weights weights) {
+ private Map<String, OwnerWeights> map;
+
+ WeightComparator(Map<String, OwnerWeights> weights) {
map = weights;
}
+
@Override
public int compare(String k1, String k2) {
OwnerWeights w1 = map.get(k1);
@@ -52,31 +54,21 @@
}
}
- StringSet files; /** paths of owned files */
- int countL1; /** number of files with control level 1 */
- int countL2; /** number of files with control level 2 */
- int countL3; /** number of files with control level 3 or more */
+ Set<String> files = new HashSet<>(); // paths of owned files
+ int countL1; // number of files with control level 1
+ int countL2; // number of files with control level 2
+ int countL3; // number of files with control level 3 or more
/** Return file counters as a compact string. */
String encodeLevelCounts() {
return "[" + countL1 + "+" + countL2 + "+" + countL3 + "]";
}
- private void init() {
- files = new StringSet();
- countL1 = 0;
- countL2 = 0;
- countL3 = 0;
- }
-
OwnerWeights(String file, int level) {
- init();
addFile(file, level);
}
- OwnerWeights() {
- init();
- }
+ OwnerWeights() {}
void addFile(String path, int level) {
// If a file is added multiple times,
@@ -94,8 +86,8 @@
}
/** Sort keys in weights map by control levels, and return keys. */
- static List<String> sortKeys(Owner2Weights weights) {
- ArrayList<String> keys = new ArrayList(weights.keySet());
+ static List<String> sortKeys(Map<String, OwnerWeights> weights) {
+ List<String> keys = new ArrayList<>(weights.keySet());
Collections.sort(keys, new WeightComparator(weights));
return keys;
}
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 503da41..04a087a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/OwnersDb.java
@@ -14,16 +14,24 @@
package com.googlesource.gerrit.plugins.findowners;
-import com.googlesource.gerrit.plugins.findowners.Util.Owner2Weights;
-import com.googlesource.gerrit.plugins.findowners.Util.String2StringSet;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.PathMatcher;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevTree;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.treewalk.TreeWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -31,40 +39,45 @@
class OwnersDb {
private static final Logger log = LoggerFactory.getLogger(OwnersDb.class);
- private Server server; // could be set to a mocked server in unit tests
+ private int numOwners = -1; // # of owners of all given files.
- private int numOwners; // # of owners of all given files.
+ String key = ""; // key to find this OwnersDb in a cache.
+ String revision = ""; // tip of branch revision, where OWENRS were found.
+ Map<String, Set<String>> dir2Globs = new HashMap<>(); // directory to file globs in the directory
+ Map<String, Set<String>> owner2Paths = new HashMap<>(); // owner email to owned dirs or file globs
+ Map<String, Set<String>> path2Owners = new HashMap<>(); // dir or file glob to owner emails
+ Set<String> readDirs = new HashSet<>(); // directories in which we have checked OWNERS
+ Set<String> stopLooking = new HashSet<>(); // directories where OWNERS has "set noparent"
- String revision; // tip of branch revision, where OWENRS were found.
- String2StringSet dir2Globs; // (directory) => file globs in the directory
- String2StringSet owner2Paths; // (owner email) => owned dirs or file globs
- String2StringSet path2Owners; // (directory or file glob) => owner emails
- StringSet readDirs; // directories in which we have checked OWNERS
- StringSet stopLooking; // directories where OWNERS has "set noparent"
+ OwnersDb() {}
- private void init(Server s) {
- numOwners = -1;
- revision = "";
- dir2Globs = new String2StringSet();
- owner2Paths = new String2StringSet();
- path2Owners = new String2StringSet();
- readDirs = new StringSet();
- stopLooking = new StringSet();
- server = (null != s) ? s : new Server();
- }
-
- OwnersDb(Server s) {
- init(s);
- }
-
- OwnersDb(Server s, String key, Repository repository,
- String branch, Collection<String> files) {
- init(s, key, repository, null, null, branch, files);
- }
-
- OwnersDb(Server s, String key, String url, String project,
- String branch, Collection<String> files) {
- init(s, key, null, url, project, branch, files);
+ OwnersDb(String key, Repository repository, String branch, Collection<String> files) {
+ this.key = key;
+ for (String fileName : files) {
+ // Find OWNERS in fileName's directory and parent directories.
+ // Stop looking for a parent directory if OWNERS has "set noparent".
+ fileName = Util.normalizedFilePath(fileName);
+ 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 content = getRepositoryFile(repository, branch, filePath);
+ if (content != null && !content.equals("")) {
+ addFile(dir + "/", dir + "/OWNERS", content.split("\\R+"));
+ }
+ if (stopLooking.contains(dir + "/") || !dir.contains("/")) {
+ break; // stop looking through parent directory
+ }
+ dir = Util.getDirName(dir); // go up one level
+ }
+ }
+ countNumOwners(files);
+ try {
+ revision = repository.getRef(branch).getObjectId().getName();
+ } catch (IOException e) {
+ log.error("Fail to get branch revision", e);
+ revision = "";
+ }
}
int getNumOwners() {
@@ -72,57 +85,54 @@
}
private void countNumOwners(Collection<String> files) {
- String2StringSet file2Owners = findOwners(files, null);
- if (null != file2Owners) {
- StringSet emails = new StringSet();
- for (String key : file2Owners.keySet()) {
- for (String owner : file2Owners.get(key)) {
- emails.add(owner);
- }
- }
+ Map<String, Set<String>> file2Owners = findOwners(files, null);
+ if (file2Owners != null) {
+ Set<String> emails = new HashSet<>();
+ file2Owners.values().forEach(emails::addAll);
numOwners = emails.size();
} else {
numOwners = owner2Paths.keySet().size();
}
}
- private static void addToMap(String2StringSet map,
- String key, String value) {
- if (null == map.get(key)) {
- map.put(key, new StringSet());
- }
- map.get(key).add(value);
- }
-
void addOwnerPathPair(String owner, String path) {
- addToMap(owner2Paths, owner, path);
- addToMap(path2Owners, path, owner);
+ Util.addToMap(owner2Paths, owner, path);
+ Util.addToMap(path2Owners, path, owner);
if (path.length() > 0 && path.charAt(path.length() - 1) != '/') {
- addToMap(dir2Globs, Util.getDirName(path) + "/", path); // A file glob.
+ Util.addToMap(dir2Globs, Util.getDirName(path) + "/", path); // A file glob.
}
}
- void addFile(String path, String file, String[] lines) {
- int n = 0;
- for (String line : lines) {
- String error = Parser.parseLine(this, path, file, line, ++n);
- if (null != error && server.getReportSyntaxError()) {
- log.warn(error);
+ void addFile(String dirPath, String filePath, String[] lines) {
+ Parser.Result result = Parser.parseFile(dirPath, filePath, lines);
+ if (result.stopLooking) {
+ stopLooking.add(dirPath);
+ }
+ for (String owner : result.owner2paths.keySet()) {
+ for (String path : result.owner2paths.get(owner)) {
+ addOwnerPathPair(owner, path);
}
}
+ if (Config.getReportSyntaxError()) {
+ result.warnings.forEach(w -> log.warn(w));
+ result.errors.forEach(w -> log.error(w));
+ }
}
private void addOwnerWeights(
- ArrayList<String> paths, ArrayList<Integer> distances,
- String file, String2StringSet file2Owners, Owner2Weights map) {
+ ArrayList<String> paths,
+ ArrayList<Integer> distances,
+ String file,
+ Map<String, Set<String>> file2Owners,
+ Map<String, OwnerWeights> map) {
for (int i = 0; i < paths.size(); i++) {
- StringSet owners = path2Owners.get(paths.get(i));
- if (null == owners) {
+ Set<String> owners = path2Owners.get(paths.get(i));
+ if (owners == null) {
continue;
}
for (String name : owners) {
- addToMap(file2Owners, file, name);
- if (null == map) {
+ Util.addToMap(file2Owners, file, name);
+ if (map == null) {
continue;
}
if (map.containsKey(name)) {
@@ -135,24 +145,23 @@
}
/** Quick method to find owner emails of every file. */
- String2StringSet findOwners(Collection<String> files) {
+ Map<String, Set<String>> findOwners(Collection<String> files) {
return findOwners(files, null);
}
/** Returns owner emails of every file and set up ownerWeights. */
- String2StringSet findOwners(Collection<String> files,
- Owner2Weights ownerWeights) {
+ Map<String, Set<String>> findOwners(
+ Collection<String> files, Map<String, OwnerWeights> ownerWeights) {
return findOwners(files.toArray(new String[0]), ownerWeights);
}
/** Returns true if path has '*' owner. */
- private boolean findStarOwner(String path, int distance,
- ArrayList<String> paths,
- ArrayList<Integer> distances) {
- StringSet owners = path2Owners.get(path);
- if (null != owners) {
+ private boolean findStarOwner(
+ String path, int distance, ArrayList<String> paths, ArrayList<Integer> distances) {
+ Set<String> owners = path2Owners.get(path);
+ if (owners != null) {
paths.add(path);
- distances.add(new Integer(distance));
+ distances.add(distance);
if (owners.contains("*")) {
return true;
}
@@ -161,12 +170,12 @@
}
/** Returns owner emails of every file and set up ownerWeights. */
- String2StringSet findOwners(String[] files, Owner2Weights ownerWeights) {
+ Map<String, Set<String>> findOwners(String[] files, Map<String, OwnerWeights> ownerWeights) {
// Returns a map of file to set of owner emails.
// If ownerWeights is not null, add to it owner to distance-from-dir;
// a distance of 1 is the lowest/closest possible distance
// (which makes the subsequent math easier).
- String2StringSet file2Owners = new String2StringSet();
+ Map<String, Set<String>> file2Owners = new HashMap<>();
for (String fileName : files) {
fileName = Util.normalizedFilePath(fileName);
String dirPath = Util.normalizedDirPath(fileName);
@@ -175,13 +184,13 @@
FileSystem fileSystem = FileSystems.getDefault();
// Collect all matched (path, distance) in all OWNERS files for
// fileName. Add them only if there is no special "*" owner.
- ArrayList<String> paths = new ArrayList<String>();
- ArrayList<Integer> distances = new ArrayList<Integer>();
+ ArrayList<String> paths = new ArrayList<>();
+ ArrayList<Integer> distances = new ArrayList<>();
boolean foundStar = false;
while (true) {
int savedSizeOfPaths = paths.size();
if (dir2Globs.containsKey(dirPath + "/")) {
- StringSet patterns = dir2Globs.get(dirPath + "/");
+ Set<String> patterns = dir2Globs.get(dirPath + "/");
for (String pat : patterns) {
PathMatcher matcher = fileSystem.getPathMatcher("glob:" + pat);
if (matcher.matches(Paths.get(dirPath + "/" + baseName))) {
@@ -201,45 +210,33 @@
}
if (foundStar // This file can be approved by anyone, no owner.
|| stopLooking.contains(dirPath + "/") // stop looking parent
- || !dirPath.contains("/") /* root */ ) {
+ || !dirPath.contains("/") /* root */) {
break;
}
if (paths.size() != savedSizeOfPaths) {
- distance++; // increase distance for each found OWNERS
+ distance++; // increase distance for each found OWNERS
}
dirPath = Util.getDirName(dirPath); // go up one level
}
if (!foundStar) {
- addOwnerWeights(paths, distances, fileName,
- file2Owners, ownerWeights);
+ addOwnerWeights(paths, distances, fileName, file2Owners, ownerWeights);
}
}
return file2Owners;
}
- private void init(
- Server s, String key, Repository repository, String url,
- String project, String branch, Collection<String> files) {
- init(s);
- for (String fileName : files) {
- // Find OWNERS in fileName's directory and parent directories.
- // Stop looking for a parent directory if OWNERS has "set noparent".
- fileName = Util.normalizedFilePath(fileName);
- String dir = Util.normalizedDirPath(fileName);
- while (!readDirs.contains(dir)) {
- readDirs.add(dir);
- String content = server.getOWNERS(dir, repository, url,
- project, branch);
- if (null != content && !content.equals("")) {
- addFile(dir + "/", dir + "/OWNERS", content.split("\\R+"));
- }
- if (stopLooking.contains(dir + "/") || !dir.contains("/")) {
- break; // stop looking through parent directory
- }
- dir = Util.getDirName(dir); // go up one level
+ /** Returns file content or empty string; uses Repository. */
+ static String getRepositoryFile(Repository repo, String branch, String file) {
+ try (RevWalk revWalk = new RevWalk(repo)) {
+ RevTree tree = revWalk.parseCommit(repo.resolve(branch)).getTree();
+ ObjectReader reader = revWalk.getObjectReader();
+ TreeWalk treeWalk = TreeWalk.forPath(reader, file, tree);
+ if (treeWalk != null) {
+ return new String(reader.open(treeWalk.getObjectId(0)).getBytes(), UTF_8);
}
+ } catch (IOException e) {
+ log.error("get file " + file, e);
}
- countNumOwners(files);
- revision = server.getBranchRevision(repository, url, project, branch);
+ return "";
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java
index e1dfa8a..57f137d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Parser.java
@@ -14,15 +14,20 @@
package com.googlesource.gerrit.plugins.findowners;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Parse lines in an OWNERS file and put them into an OwnersDb.
*
- * <p>OWNERS file syntax:<pre>
+ * <p>OWNERS file syntax:
+ *
+ * <pre>
* lines := (\s* line? \s* "\n")*
* line := "set noparent"
* | "per-file" \s+ glob \s* "=" \s* directive
@@ -34,67 +39,83 @@
* glob := [a-zA-Z0-9_-*?]+
* comment := "#" [^"\n"]*
* </pre>
- * <p> The "file:" directive is not implemented yet.
- * <p> "per-file glob = directive" applies directive only to files
- * matching glob. glob does not contain directory path.
+ *
+ * <p>The "file:" directive is not implemented yet.
+ *
+ * <p>"per-file glob = directive" applies directive only to files matching glob. glob does not
+ * contain directory path.
*/
class Parser {
- private static final Logger log = LoggerFactory.getLogger(Parser.class);
-
- static final Pattern PatComment = Pattern.compile("^ *(#.*)?$");
+ static final Pattern patComment = Pattern.compile("^ *(#.*)?$");
// TODO: have a more precise email address pattern.
- static final Pattern PatEmail = // email address or a "*"
+ static final Pattern patEmail = // email address or a "*"
Pattern.compile("^ *([^ <>@]+@[^ <>@#]+|\\*) *(#.*)?$");
- static final Pattern PatFile = Pattern.compile("^ *file:.*$");
- static final Pattern PatNoParent =
- Pattern.compile("^ *set +noparent(( |#).*)?$");
- static final Pattern PatPerFile =
- Pattern.compile("^ *per-file +([^= ]+) *= *([^ #]+).*$");
+ static final Pattern patFile = Pattern.compile("^ *file:.*$");
+ static final Pattern patNoParent = Pattern.compile("^ *set +noparent(( |#).*)?$");
+ static final Pattern patPerFile = Pattern.compile("^ *per-file +([^= ]+) *= *([^#]+).*$");
+
+ static class Result {
+ boolean stopLooking; // if this file contains set noparent
+ List<String> warnings; // warning messages
+ List<String> errors; // error messages
+ Map<String, Set<String>> owner2paths; // maps from owner email to pathGlobs
+
+ Result() {
+ stopLooking = false;
+ warnings = new ArrayList<>();
+ errors = new ArrayList<>();
+ owner2paths = new HashMap<>();
+ }
+ }
+
+ static Result parseFile(String dir, String file, String[] lines) {
+ Result result = new Result();
+ int n = 0;
+ for (String line : lines) {
+ Parser.parseLine(result, dir, file, line, ++n);
+ }
+ return result;
+ }
/**
- * Parse a line in OWNERS file and add info to OwnersDb.
+ * Parse a line in OWNERS file and add info to OwnersDb.
*
- * @param db an OwnersDb to keep parsed info.
- * @param path the path of OWNERS file.
- * @param file the OWNERS file path.
- * @param line the source line.
- * @param num the line number.
- * @return error message string or null.
+ * @param result a Result object to keep parsed info.
+ * @param dir the path to OWNERS file directory.
+ * @param file the OWNERS file path.
+ * @param line the source line.
+ * @param num the line number.
*/
- static String parseLine(OwnersDb db, String path,
- String file, String line, int num) {
+ static void parseLine(Result result, String dir, String file, String line, int num) {
// comment and file: directive are parsed but ignored.
- if (PatNoParent.matcher(line).find()) {
- db.stopLooking.add(path);
- return null;
- }
- if (PatPerFile.matcher(line).find()) {
- Matcher m = PatPerFile.matcher(line);
+ if (patNoParent.matcher(line).find()) {
+ result.stopLooking = true;
+ } else if (patPerFile.matcher(line).find()) {
+ Matcher m = patPerFile.matcher(line);
m.find();
- return parseDirective(db, path + m.group(1), file, m.group(2), num);
+ parseDirective(result, dir + m.group(1), file, m.group(2).trim(), num);
+ } else if (patFile.matcher(line).find()) {
+ result.warnings.add(warningMsg(file, num, "ignored", line));
+ } else if (patComment.matcher(line).find()) {
+ // ignore comment and empty lines.
+ } else {
+ parseDirective(result, dir, file, line, num);
}
- if (PatFile.matcher(line).find()) {
- return warningMsg(file, num, "ignored", line);
- }
- // ignore comment and empty lines.
- return (PatComment.matcher(line).find())
- ? null : parseDirective(db, path, file, line, num);
}
- private static String parseDirective(OwnersDb db, String pathGlob,
- String file, String line, int num) {
+ private static void parseDirective(
+ Result result, String pathGlob, String file, String line, int num) {
// A directive is an email address or "*".
- if (PatEmail.matcher(line).find()) {
- Matcher m = PatEmail.matcher(line);
+ if (patEmail.matcher(line).find()) {
+ Matcher m = patEmail.matcher(line);
m.find();
- db.addOwnerPathPair(m.group(1), pathGlob);
- return null;
+ Util.addToMap(result.owner2paths, m.group(1), pathGlob);
+ } else {
+ result.errors.add(errorMsg(file, num, "ignored unknown line", line));
}
- return errorMsg(file, num, "ignored unknown line", line);
}
- private static String createMsgLine(
- String prefix, String file, int n, String msg, String line) {
+ private static String createMsgLine(String prefix, String file, int n, String msg, String line) {
return prefix + file + ":" + n + ": " + msg + ": [" + line + "]";
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/RestResult.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/RestResult.java
new file mode 100644
index 0000000..b0b227c
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/RestResult.java
@@ -0,0 +1,60 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.findowners;
+
+import com.google.gson.annotations.SerializedName;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+
+/** REST API return data Java class. */
+public class RestResult {
+ @SerializedName("minOwnerVoteLevel")
+ int minOwnerVoteLevel;
+
+ @SerializedName("addDebugMsg")
+ boolean addDebugMsg;
+
+ int change;
+ int patchset;
+
+ @SerializedName("owner_revision")
+ String ownerRevision;
+
+ DebugMessages dbgmsgs;
+ SortedMap<String, String> file2owners = new TreeMap<>();
+ List<String> reviewers = new ArrayList<>();
+ List<String> owners = new ArrayList<>();
+ List<String> files = new ArrayList<>();
+
+ RestResult(int voteLevel, boolean addDebugMsg) {
+ minOwnerVoteLevel = voteLevel;
+ this.addDebugMsg = addDebugMsg;
+ if (addDebugMsg) {
+ dbgmsgs = new DebugMessages();
+ dbgmsgs.path2owners = new TreeMap<>();
+ dbgmsgs.owner2paths = new TreeMap<>();
+ }
+ }
+
+ static class DebugMessages {
+ String user;
+ String project;
+ String branch;
+ SortedMap<String, String> path2owners;
+ SortedMap<String, String> owner2paths;
+ };
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Server.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Server.java
deleted file mode 100644
index 98a75aa..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Server.java
+++ /dev/null
@@ -1,304 +0,0 @@
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.findowners;
-
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Change.Status;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.AccountAccess;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.rules.StoredValues;
-import com.google.gerrit.server.change.RevisionResource;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gson.JsonArray;
-import com.google.gson.JsonElement;
-import com.google.gson.JsonObject;
-import com.google.gwtorm.server.OrmException;
-import com.googlecode.prolog_cafe.lang.Prolog;
-import com.googlesource.gerrit.plugins.findowners.Util.String2Integer;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
-import java.io.IOException;
-import java.net.URLEncoder;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Map;
-import org.eclipse.jgit.lib.ObjectReader;
-import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevTree;
-import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.treewalk.TreeWalk;
-import org.eclipse.jgit.treewalk.filter.PathFilter;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/** Wrapper of Gerrit server functions. */
-class Server {
- // Usage: construct one instance and init for each change id.
- private static final Logger log = LoggerFactory.getLogger(Server.class);
-
- // Accept both "Exempt-" and "Exempted-".
- private static final String EXEMPT_MESSAGE1 = "Exempt-From-Owner-Approval:";
- private static final String EXEMPT_MESSAGE2 = "Exempted-From-Owner-Approval:";
-
- private Prolog engine; // Gerrit Prolog engine
- String url; // Gerrit server URL, could be changed in mocked server
-
- int change;
- int patchset; // patchset number
- String project;
- String branch;
- String error; // default init to null
- Collection<PatchSetApproval> approvals; // only used with Prolog engine
-
- Server() {}
-
- Server(Prolog p) {
- engine = p;
- ChangeData data = StoredValues.CHANGE_DATA.get(engine);
- change = data.getId().get();
- try {
- patchset = data.currentPatchSet().getId().get();
- } catch (OrmException e) {
- log.error("Cannot get patchset: " + e);
- patchset = 1;
- }
- Change c = StoredValues.getChange(engine);
- project = c.getProject().get();
- // NOTE: repository.getBranch() returns incorrect "master"
- branch = c.getDest().get(); // e.g. "refs/heads/BetaBranch"
- try {
- approvals = data.currentApprovals();
- } catch (OrmException e) {
- log.error("Cannot get approvals: " + e);
- approvals = new ArrayList<PatchSetApproval>();
- }
- }
-
- int getMinOwnerVoteLevel() {
- return Config.getMinOwnerVoteLevel(new Project.NameKey(project));
- }
-
- boolean getAddDebugMsg() {
- return Config.getAddDebugMsg();
- }
-
- boolean traceServerMsg() {
- return Config.traceServerMsg();
- }
-
- boolean getReportSyntaxError() {
- return Config.getReportSyntaxError();
- }
-
- /** Returns a revision's change review status. */
- Status getStatus(RevisionResource resource) {
- return resource.getChange().getStatus();
- }
-
- /** Sets change number; retrieves other parameters from REST API. */
- void setChangeId(String url, int change) {
- this.url = url;
- this.change = change;
- String request = url + "changes/?q=" + change + "&o=CURRENT_REVISION";
- JsonArray arr = Util.getHTTPJsonArray(request);
- if (null == arr || arr.size() != 1) {
- error = "Failed request: " + request;
- return;
- }
- JsonObject obj = arr.get(0).getAsJsonObject();
- project = obj.get("project").getAsString();
- branch = obj.get("branch").getAsString();
- String revisionString = obj.get("current_revision").getAsString();
- JsonObject revisions = obj.get("revisions").getAsJsonObject();
- JsonObject revInfo = revisions.get(revisionString).getAsJsonObject();
- patchset = revInfo.get("_number").getAsInt();
- }
-
- /** Returns error message if patchsetNum has invalid value. */
- String setPatchId(String patchsetNum) {
- if (null != patchsetNum) {
- int n = Integer.parseInt(patchsetNum);
- if (n < 1 || n > patchset) {
- return "Invalid patchset parameter: " + patchsetNum + "; must be 1"
- + ((1 != patchset) ? (" to " + patchset) : "");
- }
- patchset = n;
- }
- return null;
- }
-
- /** Returns a map from reviewer email to vote value; uses Prolog engine. */
- String2Integer getVotes() {
- ChangeData data = StoredValues.CHANGE_DATA.get(engine);
- ReviewDb db = StoredValues.REVIEW_DB.get(engine);
- String2Integer map = new String2Integer();
- AccountAccess ac = db.accounts();
- for (PatchSetApproval p : approvals) {
- if (p.getValue() != 0) {
- int id = p.getAccountId().get();
- try {
- Account a = ac.get(new Account.Id(id));
- String email = a.getPreferredEmail();
- map.put(email, new Integer(p.getValue()));
- } catch (OrmException e) {
- log.error("Cannot get email address of account id: " + id + " " + e);
- }
- }
- }
- return map;
- }
-
- /** Returns changed files, uses Prolog engine or url REST API. */
- Collection<String> getChangedFiles() {
- if (null != engine) { // Get changed files faster from StoredValues.
- try {
- return StoredValues.CHANGE_DATA.get(engine).currentFilePaths();
- } catch (OrmException e) {
- log.error("OrmException in getChangedFiles: " + e);
- return new StringSet();
- }
- }
- String request =
- url + "changes/" + change + "/revisions/" + patchset + "/files";
- JsonObject map = Util.getHTTPJsonObject(request, false);
- StringSet result = new StringSet();
- for (Map.Entry<String, JsonElement> entry : map.entrySet()) {
- String key = entry.getKey();
- if (!key.equals("/COMMIT_MSG")) { // ignore commit message
- result.add(key);
- // If a file was moved then we need approvals for old directory.
- JsonObject attr = entry.getValue().getAsJsonObject();
- if (null != attr && null != attr.get("old_path")) {
- result.add(attr.get("old_path").getAsString());
- }
- }
- }
- return result;
- }
-
- /** Returns reviewer emails got from url REST API. */
- JsonArray getReviewers() {
- String request = url + "changes/" + change + "/reviewers";
- JsonArray reviewers = Util.getHTTPJsonArray(request);
- JsonArray result = new JsonArray();
- int numReviewers = reviewers.size();
- for (int i = 0; i < numReviewers; i++) {
- JsonObject map = reviewers.get(i).getAsJsonObject();
- result.add(map.get("email").getAsString() + " []");
- }
- return result;
- }
-
- /** Returns file content or empty string; uses Repository. */
- String getRepositoryFile(Repository repo, String branch, String file) {
- try (RevWalk revWalk = new RevWalk(repo)) {
- RevTree tree = revWalk.parseCommit(repo.resolve(branch)).getTree();
- try (TreeWalk treeWalk = new TreeWalk(repo)) {
- try (ObjectReader reader = repo.newObjectReader()) {
- treeWalk.addTree(tree);
- treeWalk.setRecursive(true);
- treeWalk.setFilter(PathFilter.create(file));
- if (treeWalk.next()) {
- return new String(reader.open(treeWalk.getObjectId(0)).getBytes());
- }
- }
- }
- } catch (IOException e) {
- log.error("get file " + file + ": " + e);
- }
- return "";
- }
-
- /** Returns OWNERS file content; uses Repository or url REST API. */
- String getOWNERS(String dir, Repository repository, String url,
- String project, String branch) {
- // e.g. dir = ./d1/d2
- String filePath = (dir + "/OWNERS").substring(2); // remove "./"
- if (null != repository) {
- return getRepositoryFile(repository, branch, filePath);
- } else {
- String requestUrl = url + "projects/" + project
- + "/branches/" + branch + "/files/"
- + URLEncoder.encode(filePath) + "/content";
- return Util.getHTTPBase64Content(requestUrl);
- }
- }
-
- /** Returns the revision string of the tip of target branch. */
- String getBranchRevision(Repository repository, String url,
- String project, String branch) {
- if (null != repository) {
- try {
- return repository.getRef(
- repository.getBranch()).getObjectId().getName();
- } catch (IOException e) {
- log.error("Fail to get branch revision: " + e);
- }
- } else {
- JsonObject obj = Util.getHTTPJsonObject(
- url + "projects/" + project + "/branches/" + branch, true);
- // cannot get revision of branch "refs/meta/config".
- if (null != obj && null != obj.get("revision")) {
- return obj.get("revision").getAsString();
- }
- }
- return "";
- }
-
- /** Returns true if exempt from owner approval; uses Prolog engine. */
- boolean isExemptFromOwnerApproval() {
- if (null == engine) {
- return true;
- }
- try {
- ChangeData data = StoredValues.CHANGE_DATA.get(engine);
- String message = data.commitMessage();
- if (message.contains(EXEMPT_MESSAGE1)
- || message.contains(EXEMPT_MESSAGE2)) {
- return true;
- }
- } catch (IOException | OrmException e) {
- log.error("Cannot get commit message: " + e);
- return true; // exempt from owner approval due to lack of data
- }
- // Abandoned and merged changes do not need approval again.
- Status status = StoredValues.getChange(engine).getStatus();
- return (status == Status.ABANDONED || status == Status.MERGED);
- }
-
- /** Returns a cached or new OwnersDb. */
- OwnersDb getCachedOwnersDb() {
- if (null != engine) { // Get changed files faster from StoredValues.
- Repository repository = StoredValues.REPOSITORY.get(engine);
- String dbKey = Cache.makeKey(change, patchset, branch);
- return Cache.get(this, dbKey, repository, branch, getChangedFiles());
- }
- String key = Cache.makeKey(change, patchset, branch);
- return Cache.get(this, key, url, project, branch, getChangedFiles());
- }
-
- /** Returns a debug message string, for server side logging. */
- String genDebugMsg(OwnersDb db) {
- return (null == url ? "" : ("\n## url=" + url))
- + "\n## change=" + change + ", patchset=" + patchset
- + ", project=" + project + ", branch=" + branch
- + "\n## changedFiles=" + getChangedFiles()
- + "\nnumOwners=" + db.getNumOwners()
- + ", minVoteLevel=" + getMinOwnerVoteLevel()
- + ", approvals=" + getVotes();
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Servlet.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Servlet.java
deleted file mode 100644
index 590290a..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Servlet.java
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.findowners;
-
-import com.google.inject.servlet.ServletModule;
-
-/** Plugin HTTP Servlet module */
-public class Servlet extends ServletModule {
- protected void configureServlets() {
- serve("/change/*").with(GetChange.class);
- }
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/findowners/Util.java b/src/main/java/com/googlesource/gerrit/plugins/findowners/Util.java
index e0ca853..76a0e53 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/findowners/Util.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/findowners/Util.java
@@ -14,89 +14,22 @@
package com.googlesource.gerrit.plugins.findowners;
-import com.google.gson.JsonArray;
-import com.google.gson.JsonObject;
-import com.google.gson.JsonParser;
-import com.googlesource.gerrit.plugins.findowners.Util.Owner2Weights;
-import com.googlesource.gerrit.plugins.findowners.Util.String2StringSet;
+import com.google.common.collect.Ordering;
import java.io.File;
-import java.io.IOException;
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.net.URLConnection;
-import java.util.ArrayList;
-import java.util.Base64;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
import java.util.HashSet;
-import java.util.List;
-import java.util.Scanner;
+import java.util.Map;
import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import java.util.SortedMap;
+import java.util.TreeMap;
/** Utility classes and functions. */
class Util {
- private static final Logger log = LoggerFactory.getLogger(Util.class);
-
- static class Owner2Weights extends HashMap<String, OwnerWeights> {}
- static class String2Integer extends HashMap<String, Integer> {}
- static class String2String extends HashMap<String, String> {}
- static class String2StringSet extends HashMap<String, StringSet> {}
- static class StringSet extends HashSet<String> {}
-
- /** Removes extra "/" in url. */
- static String normalizeURL(String url) {
- return url.replace(":///", "://"); // Assuming only one ":///" in url.
- }
-
/** Strips REST magic prefix line. */
static String stripMagicPrefix(String data) {
final String magic = ")]}'\n";
return data.startsWith(magic) ? data.substring(magic.length()) : data;
}
- /** Issues Gerrit REST API GET command. */
- private static String getHTTP(String urlString, boolean ignoreIOException) {
- urlString = normalizeURL(urlString);
- try {
- URLConnection conn = new URL(urlString).openConnection();
- Scanner scanner = new Scanner(conn.getInputStream());
- String responseBody = scanner.useDelimiter("\\A").next();
- return stripMagicPrefix(responseBody);
- } catch (MalformedURLException e) {
- log.error("Malformed URL: " + urlString);
- } catch (IOException e) {
- // Not an error if looking for an OWNERS file
- // or revision info in the "refs/meta/config" branch.
- if (!ignoreIOException) {
- log.error("IOException URL: " + urlString);
- }
- }
- return null;
- }
-
- /** Issues Gerrit REST API GET; converts result to JsonObject. */
- static JsonObject getHTTPJsonObject(String url, boolean ignoreIOException) {
- String data = getHTTP(url, ignoreIOException);
- return (null == data) ? new JsonObject()
- : new JsonParser().parse(data).getAsJsonObject();
- }
-
- /** Issues Gerrit REST API GET; converts result to JsonArray. */
- static JsonArray getHTTPJsonArray(String url) {
- String data = getHTTP(url, false);
- return (null == data) ? new JsonArray()
- : new JsonParser().parse(data).getAsJsonArray();
- }
-
- /** Issues Gerrit REST API GET; decodes base64 content. */
- static String getHTTPBase64Content(String url) {
- String data = getHTTP(url, true);
- return (null == data) ? "" : new String(Base64.getDecoder().decode(data));
- }
-
static String getDirName(String path) {
return new File(path).getParent();
}
@@ -110,23 +43,21 @@
}
static boolean parseBoolean(String s) {
- return (null != s) && (s.equals("1")
- || s.equalsIgnoreCase("yes") || Boolean.parseBoolean(s));
+ return (s != null) && (s.equals("1") || s.equalsIgnoreCase("yes") || Boolean.parseBoolean(s));
}
- static List<String> sort(Set<String> names) {
- List<String> list = new ArrayList<String>(names);
- Collections.sort(list);
- return list;
- }
-
- static JsonArray newJsonArrayFromStrings(Collection<String> names) {
- JsonArray result = new JsonArray();
- List<String> list = new ArrayList<String>(names);
- Collections.sort(list);
- for (String name : list) {
- result.add(name);
+ static SortedMap<String, String> makeSortedMap(Map<String, Set<String>> map) {
+ SortedMap<String, String> result = new TreeMap<>();
+ for (String key : Ordering.natural().sortedCopy(map.keySet())) {
+ result.put(key, String.join(" ", Ordering.natural().sortedCopy(map.get(key))));
}
return result;
}
+
+ static void addToMap(Map<String, Set<String>> map, String key, String value) {
+ if (map.get(key) == null) {
+ map.put(key, new HashSet<>());
+ }
+ map.get(key).add(value);
+ }
}
diff --git a/src/main/java/find_owners/PRED_check_owner_approval_2.java b/src/main/java/find_owners/PRED_check_owner_approval_2.java
index 52c9338..082b50e 100644
--- a/src/main/java/find_owners/PRED_check_owner_approval_2.java
+++ b/src/main/java/find_owners/PRED_check_owner_approval_2.java
@@ -21,18 +21,13 @@
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
import com.googlesource.gerrit.plugins.findowners.Checker;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
- * 'check_owner_approval'(+N, R) sets R to -1, 0, or 1,
- * if owner approval is missing, unneeded, or satisfied.
+ * 'check_owner_approval'(+N, R) sets R to -1, 0, or 1, if owner approval is missing, unneeded, or
+ * satisfied.
*/
public class PRED_check_owner_approval_2 extends Predicate.P2 {
- private static final Logger log =
- LoggerFactory.getLogger(PRED_check_owner_approval_2.class);
-
public PRED_check_owner_approval_2(Term a1, Term a2, Operation n) {
arg1 = a1;
arg2 = a2;
diff --git a/src/main/resources/Documentation/rest-api.md b/src/main/resources/Documentation/rest-api.md
index 2e792b6..34152d3 100644
--- a/src/main/resources/Documentation/rest-api.md
+++ b/src/main/resources/Documentation/rest-api.md
@@ -2,13 +2,13 @@
===========================
Any Gerrit UI clients or tools can use the
-`plugins/find-owners/change/<id>` API to get
+`/changes/<id>/owners` API to get
OWNERS information of a change.
### Request
```bash
-GET /plugins/find-owners/change/<id> HTTP/1.0
+GET /changes/<id>/owners HTTP/1.0
```
The `<id>` is a Gerrit change ID. This API can have two parameters:
@@ -22,7 +22,7 @@
For example,
```bash
-http://.../plugins/find-owners/change/29?debug=true&patchset=3
+http://<gerrit_server>/changes/29/owners?debug=true&patchset=3
```
### Response
@@ -46,18 +46,23 @@
Due to caching this revision might be behind recent branches changes.
* **dbgmsgs**: returned only when addDebugMsg is true,
- a set of debugging messages including project name, branch name,
- server address, etc.
+ a JSON object with the following members:
-* **path2owners**: returned only when addDebugMsg is true,
- a map from directory path or file glob to a string of owner emails
- separated by space. Note that `*` is a special owner email address.
- It means that there is no owner and anyone can be the owner.
- Included directories are those affected by the change revision.
+ * **user**: the change's creator.
-* **owner2paths**: returned only when addDebugMsg is true,
- a map from owner email to directory path or file glob.
- This is opposite to the path2owners map.
+ * **project**: the change's project name.
+
+ * **branch**: the change's destination brach name.
+
+ * **path2owners**:
+ a map from directory path or file glob to a string of owner emails
+ separated by space. Note that `*` is a special owner email address.
+ It means that there is no owner and anyone can be the owner.
+ Included directories are those affected by the change revision.
+
+ * **owner2paths**:
+ a map from owner email to directory path or file glob.
+ This is opposite to the path2owners map.
* **file2owners**: a map from each file in the change patchset to
the file owner emails, separated by space.
diff --git a/src/main/resources/static/find-owners.js b/src/main/resources/static/find-owners.js
index 2e2c17c..c38471f 100644
--- a/src/main/resources/static/find-owners.js
+++ b/src/main/resources/static/find-owners.js
@@ -343,11 +343,9 @@
getReviewers(changeId, popupWindow);
}
function callServer(callBack) {
- // Use either the revision post API or plugin get API.
- // Only pass changeId, let server get current patch set,
- // project and branch info.
- c.call({change: changeId}, showFindOwnersResults);
- // self.get('change/' + changeId, showFindOwnersResults);
+ // Use the plugin REST API; pass only changeId;
+ // let server get current patch set, project and branch info.
+ Gerrit.get('changes/' + changeId + '/owners', showFindOwnersResults);
}
callServer(showFindOwnersResults);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/ActionTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/ActionTest.java
deleted file mode 100644
index 373e48a..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/ActionTest.java
+++ /dev/null
@@ -1,58 +0,0 @@
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.findowners;
-
-import static com.google.common.truth.Truth.assertThat;
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
-import com.google.gson.JsonObject;
-import com.googlesource.gerrit.plugins.findowners.Util.String2String;
-import org.junit.Before;
-import org.junit.Test;
-
-/** Test Action class */
-public class ActionTest {
- private MockedServer server;
- private Action finder;
- private Gson gs;
-
- @Before
- public void setUp() {
- finder = new Action("http://mocked:8888/", null);
- server = new MockedServer();
- finder.setServer(server);
- gs = new GsonBuilder().setPrettyPrinting().disableHtmlEscaping().create();
- }
-
- @Test
- public void getChangeDataTest() {
- JsonObject obj = finder.getChangeData(23, new String2String());
- // Write expected output as a more readable string literal,
- // remove all ' ', then use '\'' for '\"' and ' ' for '\n'.
- String expected = "{ 'minOwnerVoteLevel':1, 'addDebugMsg':true, "
- + "'change':23, 'patchset':3, 'owner_revision':'', "
- + "'dbgmsgs':{ 'user':'?', 'project':'projectA', "
- + "'branch':'master', 'server':'http://mocked:8888/' }, "
- + "'path2owners':{}, 'owner2paths':{}, 'file2owners':{}, "
- + "'reviewers':[], 'owners':[], 'files':[ './README', "
- + "'./d1/test.c', './d2/t.txt' ] }";
- String result = gs.toJson(obj).replaceAll(" ", "");
- expected = expected.replaceAll(" ", "\n");
- expected = expected.replaceAll("'", "\"");
- assertThat(result).isEqualTo(expected);
- }
-
- // TODO: test getChangeData with non-trivial parameters
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/CheckerTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/CheckerTest.java
deleted file mode 100644
index bd87a0e..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/CheckerTest.java
+++ /dev/null
@@ -1,119 +0,0 @@
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.findowners;
-
-import static com.google.common.truth.Truth.assertThat;
-import com.googlesource.gerrit.plugins.findowners.Util.String2Integer;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
-import org.junit.Before;
-import org.junit.Test;
-
-/** Test Checker class */
-public class CheckerTest {
- private MockedServer server;
-
- @Before
- public void setUp() {
- server = new MockedServer();
- }
-
- @Test
- public void findOwnersInVotesTest() {
- Checker c = new Checker(server, 2);
- StringSet owners = new StringSet();
- String2Integer votes = new String2Integer();
- // no owner, default is false.
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(false);
- // has owner, no vote, default is false.
- owners.add("xyz@google.com");
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(false);
- // has "*" owner, no owner vote is needed
- owners.add("*");
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(true);
- // again, no owner means no
- owners = new StringSet();
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(false);
- // two owners, but only +1
- owners.add("abc@google.com");
- owners.add("xyz@google.com");
- votes.put("xyz@google.com", 1);
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(false);
- // one owner +2 vote is enough
- votes.put("xyz@google.com", 2);
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(true);
- // two +1 votes is not the same as one +2
- votes.put("abc@google.com", 1);
- votes.put("xyz@google.com", 1);
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(false);
- votes.put("xyz@google.com", 2);
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(true);
- // one owner -1 is a veto.
- votes.put("abc@google.com", -1);
- assertThat(c.findOwnersInVotes(owners, votes)).isEqualTo(false);
- }
-
- void addOwnersInfo(MockedOwnersDb db) {
- String[] emails = {"abc@google.com", "xyz@google.com"};
- StringSet dirs = new StringSet();
- StringSet owners = new StringSet();
- dirs.add("./d1");
- for (String e : emails) {
- db.owner2Paths.put(e, dirs);
- owners.add(e);
- }
- db.mockedFile2Owners.put("./d1/f1.c", owners);
- }
-
- @Test
- public void findApprovalOwnersDbTest() {
- Checker c = new Checker(server, 2);
- MockedOwnersDb db = new MockedOwnersDb();
- assertThat(c.findApproval(db)).isEqualTo(0); // no owners info
- addOwnersInfo(db); // add one file and two owners
- assertThat(db.getNumOwners()).isEqualTo(2);
- assertThat(db.findOwners(null).size()).isEqualTo(1);
- assertThat(db.findOwners(null).get("f1")).isEqualTo(null);
- assertThat(db.findOwners(null).get("./d1/f1.c").size()).isEqualTo(2);
- assertThat(c.findApproval(db)).isEqualTo(-1);
- server.votes.put("abc@google.com", 1);
- // sever has minOwnerVoteLevel 1, but checker requires 2.
- assertThat(server.getMinOwnerVoteLevel()).isEqualTo(1);
- assertThat(c.findApproval(db)).isEqualTo(-1); // vote 1 is not enough
- c = new Checker(server, 1);
- assertThat(c.findApproval(db)).isEqualTo(1);
- server.votes.put("xyz@google.com", -1);
- assertThat(c.findApproval(db)).isEqualTo(-1); // an owner's veto
- }
-
- @Test
- public void findApprovalTest() {
- Checker c = new Checker(server, 1);
- // default mocked, not exempted from owner approval
- assertThat(server.isExemptFromOwnerApproval()).isEqualTo(false);
- // not exempted, but no owner in mocked OwnersDb
- assertThat(c.findApproval()).isEqualTo(0);
- server.exemptFromOwnerApproval = true;
- assertThat(server.isExemptFromOwnerApproval()).isEqualTo(true);
- // exempted, no owner, should be 0, not 1 or -1.
- assertThat(c.findApproval()).isEqualTo(0);
- server.exemptFromOwnerApproval = false;
- // add mocked owners, no vote, should be -1, not approved.
- addOwnersInfo(server.ownersDb);
- assertThat(c.findApproval()).isEqualTo(-1);
- // add vote, should be approved now.
- server.votes.put("abc@google.com", 1);
- assertThat(c.findApproval()).isEqualTo(1);
- }
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
new file mode 100644
index 0000000..75e1e16
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/FindOwnersIT.java
@@ -0,0 +1,270 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.findowners;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.extensions.api.changes.SubmitInput;
+import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.common.ChangeInfo;
+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.server.change.ChangeResource;
+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 {
+
+ @Test
+ public void getOwnersTest() throws Exception {
+ ChangeInfo info1 = newChangeInfo("test1 GetOwners");
+ ChangeInfo info2 = newChangeInfo("test2 GetOwners");
+ assertThat(info2._number).isEqualTo(info1._number + 1);
+ String expected =
+ ")]}' { minOwnerVoteLevel:1, addDebugMsg:false, change:"
+ + info1._number
+ + ", patchset:1, file2owners:{}, reviewers:[], owners:[], files:[] }";
+ Cache cache = Cache.getInstance().init(0, 10); // reset, no Cache
+ assertThat(cache.size()).isEqualTo(0L);
+ // GetOwners GET API
+ assertThat(getOwnersResponse(info1)).isEqualTo(expected);
+ assertThat(cache.size()).isEqualTo(0L);
+ // find-owners GET API
+ assertThat(getFindOwnersResponse(info1)).isEqualTo(expected);
+ cache.init(10, 5); // create the Cache
+ assertThat(cache.size()).isEqualTo(0L);
+ assertThat(getOwnersResponse(info1)).isEqualTo(expected);
+ assertThat(getFindOwnersResponse(info1)).isEqualTo(expected);
+ assertThat(cache.size()).isEqualTo(1L);
+ }
+
+ @Test
+ public void ownersFile1Test() throws Exception {
+ // Create 1st OWNERS file, this change does not have owner.
+ PushOneCommit.Result c1 = createChange("add OWNERS", "OWNERS", "x@x\na@a\n");
+ assertThat(getOwnersResponse(c1)).contains("owners:[], files:[ OWNERS ]");
+ // Create another "t.c" file, which has no owner because c1 is not submitted yet.
+ PushOneCommit.Result c2 = createChange("add t.c", "t.c", "##");
+ assertThat(getOwnersResponse(c2)).contains("owners:[], files:[ t.c ]");
+ // Change of "t.c" file has owner after c1 is submitted.
+ approveSubmit(c1);
+ assertThat(getOwnersResponse(c2)).contains("owners:[ a@a[1+0+0], x@x[1+0+0] ], files:[ t.c ]");
+ // A submitted change gets owners info from current repository.
+ assertThat(getOwnersResponse(c1))
+ .contains("owners:[ a@a[1+0+0], x@x[1+0+0] ], files:[ OWNERS ]");
+ // Check all fields in response.
+ String expectedTail =
+ "path2owners:{ ./:a@ax@x }, owner2paths:{ a@a:./, x@x:./ } }"
+ + ", file2owners:{ ./t.c:a@ax@x }, reviewers:[], owners:[ "
+ + "a@a[1+0+0], x@x[1+0+0] ], files:[ t.c ] }";
+ assertThat(getOwnersDebugResponse(c2)).contains(expectedTail);
+ }
+
+ @Test
+ public void ownersFile2Test() throws Exception {
+ // Add OWNERS file, this test does not inherit files created in ownersFile1Test.
+ addFile("add OWNERS", "OWNERS", "per-file *.c=x@x\na@a\nc@c\nb@b\n");
+ // Add "t.c" file, which has per-file owner x@x, not a@a, b@b, c@c.
+ PushOneCommit.Result c2 = createChange("add t.c", "t.c", "Hello!");
+ assertThat(getOwnersResponse(c2)).contains("owners:[ x@x[1+0+0] ], files:[ t.c ]");
+ // Add "t.txt" file, which has new owners.
+ PushOneCommit.Result c3 = createChange("add t.txt", "t.txt", "Test!");
+ assertThat(getOwnersResponse(c3))
+ .contains("owners:[ a@a[1+0+0], b@b[1+0+0], c@c[1+0+0] ], files:[ t.txt ]");
+ }
+
+ @Test
+ public void subOwnersFileTest() throws Exception {
+ // Add OWNERS file in root and subdirectories.
+ addFile("add OWNERS", "OWNERS", "x@x\n");
+ addFile("add d1/OWNERS", "d1/OWNERS", "a@a\n");
+ addFile("add d2/OWNERS", "d2/OWNERS", "y@y\n");
+ addFile("add d3/OWNERS", "d3/OWNERS", "b@b\nset noparent\n");
+ // Add "t.c" file, which is not owned by subdirectory owners.
+ PushOneCommit.Result c2 = createChange("add t.c", "t.c", "Hello!");
+ assertThat(getOwnersResponse(c2)).contains("owners:[ x@x[1+0+0] ], files:[ t.c ]");
+ // Add "d1/t.c" file, which is owned by ./d1 and root owners.
+ PushOneCommit.Result c3 = createChange("add d1/t.c", "d1/t.c", "Hello!");
+ assertThat(getOwnersResponse(c3))
+ .contains("owners:[ a@a[1+0+0], x@x[0+1+0] ], files:[ d1/t.c ]");
+ // Add "d2/t.c" file, which is owned by ./d2 and root owners.
+ PushOneCommit.Result c4 = createChange("add d2/t.c", "d2/t.c", "Hello!");
+ assertThat(getOwnersResponse(c4))
+ .contains("owners:[ y@y[1+0+0], x@x[0+1+0] ], files:[ d2/t.c ]");
+ // Add "d2/d1/t.c" file, which is owned by ./d2 and root owners.
+ PushOneCommit.Result c5 = createChange("add d2/d1/t.c", "d2/d1/t.c", "Hello!");
+ assertThat(getOwnersResponse(c5))
+ .contains("owners:[ y@y[1+0+0], x@x[0+1+0] ], files:[ d2/d1/t.c ]");
+ // Add "d3/t.c" file, which is owned only by ./d3 owners due to "set noparent".
+ PushOneCommit.Result c6 = createChange("add d3/t.c", "d3/t.c", "Hello!");
+ assertThat(getOwnersResponse(c6)).contains("owners:[ b@b[1+0+0] ], files:[ d3/t.c ]");
+ // Add "d3/d1/t.c" file, which is owned only by ./d3 owners due to "set noparent".
+ PushOneCommit.Result c7 = createChange("add d3/d1/t.c", "d3/d1/t.c", "Hello!");
+ assertThat(getOwnersResponse(c7)).contains("owners:[ b@b[1+0+0] ], files:[ d3/d1/t.c ]");
+ }
+
+ @Test
+ public void requestErrorTest() throws Exception {
+ PushOneCommit.Result c1 = createChange("add t.c", "t.c", "##");
+ assertThat(getOwnersResponse(c1)).contains("owners:[], files:[ t.c ]");
+ int id = c1.getChange().getId().get();
+ // Correct change id.
+ String result = userRestSession.get("/changes/" + id + "/owners").getEntityContent();
+ assertThat(filteredJson(result)).contains("owners:[], files:[ t.c ]");
+ // Wrong change number, 404 not found.
+ RestResponse response = userRestSession.get("/changes/" + (id + 1) + "/owners");
+ assertThat(response.getStatusCode()).isEqualTo(404);
+ assertThat(response.getEntityContent()).isEqualTo("Not found: " + (id + 1));
+ // Wrong request parameter, 400 not a valid option
+ response = userRestSession.get("/changes/" + id + "/owners?xyz=3");
+ assertThat(response.getStatusCode()).isEqualTo(400);
+ assertThat(response.getEntityContent()).isEqualTo("\"--xyz\" is not a valid option");
+ // Wrong patchset parameter, no content
+ response = userRestSession.get("/changes/" + id + "/owners?patchset=2");
+ assertThat(response.getStatusCode()).isEqualTo(204);
+ assertThat(response.hasContent()).isFalse();
+ }
+
+ @Test
+ public void accountTest() throws Exception {
+ String[] myTestEmails = {"abc@g.com", "abc+xyz@g.com", "xyz@g.com"};
+ for (String email : myTestEmails) {
+ Account.Id id = accounts.create("User" + email, email, "FullName" + email).getId();
+ // Action.getReviewers uses accountCache to get email address.
+ assertThat(accountCache.get(id).getAccount().getPreferredEmail()).isEqualTo(email);
+ // Checker.getVotes uses AccountAccess to get email address.
+ assertThat(db.accounts().get(id).getPreferredEmail()).isEqualTo(email);
+ }
+ }
+
+ @Test
+ public void projectTest() throws Exception {
+ RestResponse response = adminRestSession.get("/projects/?d");
+ String content = response.getEntityContent();
+ // Predefined projects: "All-Projects", "All-Users",
+ // "com.googlesource.gerrit.plugins.findowners.FindOwnersIT_projectTest_project"
+ assertThat(content).contains("\"id\": \"All-Projects\",");
+ assertThat(content).contains("\"id\": \"All-Users\",");
+ assertThat(content).contains(idProject("projectTest", "project"));
+ assertThat(content).doesNotContain(idProject("projectTest", "ProjectA"));
+ createProject("ProjectA");
+ response = adminRestSession.get("/projects/?d");
+ assertThat(response.getEntityContent()).contains(idProject("projectTest", "ProjectA"));
+ }
+
+ @Test
+ public void actionApplyTest() throws Exception {
+ Cache cache = Cache.getInstance().init(0, 10);
+ assertThat(cache.size()).isEqualTo(0);
+ // TODO: create ChangeInput in a new project.
+ ChangeInfo changeInfo = newChangeInfo("test Action.apply");
+ ChangeResource cr = parseChangeResource(changeInfo.changeId);
+ Action.Parameters param = new Action.Parameters();
+ Action action = new Action(null, null, changeDataFactory, accountCache, repoManager);
+ Response<RestResult> response = action.apply(db, cr, param);
+ RestResult result = response.value();
+ verifyRestResult(result, 1, 1, changeInfo._number, false);
+ param.debug = true;
+ response = action.apply(db, cr, param);
+ result = response.value();
+ verifyRestResult(result, 1, 1, changeInfo._number, true);
+ assertThat(result.dbgmsgs.user).isEqualTo("?");
+ assertThat(result.dbgmsgs.project).isEqualTo(changeInfo.project);
+ // changeInfo.branch is "master" but result.dbgmsgs.branch is "refs/heads/master".
+ assertThat(result.dbgmsgs.branch).contains(changeInfo.branch);
+ assertThat(result.dbgmsgs.path2owners).isEmpty();
+ assertThat(result.dbgmsgs.owner2paths).isEmpty();
+ assertThat(result.file2owners).isEmpty();
+ assertThat(result.reviewers).isEmpty();
+ assertThat(result.owners).isEmpty();
+ assertThat(result.files).isEmpty();
+ // TODO: find expected value of ownerRevision.
+ assertThat(cache.size()).isEqualTo(0);
+ }
+
+ private ChangeInfo newChangeInfo(String subject) throws Exception {
+ // should be called with different subject
+ ChangeInput in = new ChangeInput();
+ in.project = project.get();
+ in.branch = "master";
+ in.subject = subject;
+ in.topic = "test empty change";
+ in.status = ChangeStatus.NEW;
+ return gApi.changes().create(in).get();
+ }
+
+ private String getFindOwnersResponse(ChangeInfo info) throws Exception {
+ return filteredJson(
+ userRestSession.get("/changes/" + info._number + "/revisions/1/find-owners"));
+ }
+
+ private String getOwnersResponse(ChangeInfo info) throws Exception {
+ return filteredJson(userRestSession.get("/changes/" + info._number + "/owners"));
+ }
+
+ private String getOwnersResponse(PushOneCommit.Result change) throws Exception {
+ return filteredJson(userRestSession.get("/changes/" + change.getChangeId() + "/owners"));
+ }
+
+ private String getOwnersDebugResponse(PushOneCommit.Result change) throws Exception {
+ return filteredJson(
+ userRestSession.get("/changes/" + change.getChangeId() + "/owners?debug=1"));
+ }
+
+ private void approveSubmit(PushOneCommit.Result change) throws Exception {
+ approve(change.getChangeId());
+ gApi.changes().id(change.getChangeId()).current().submit(new SubmitInput());
+ }
+
+ private void addFile(String subject, String file, String content) throws Exception {
+ approveSubmit(createChange(subject, file, content));
+ }
+
+ // Remove '"' and space; replace '\n' with ' '; ignore unpredictable "owner_revision".
+ private static String filteredJson(String json) {
+ return json.replaceAll("[\" ]*", "").replace('\n', ' ').replaceAll("owner_revision:[^ ]* ", "");
+ }
+
+ private static String filteredJson(RestResponse response) throws Exception {
+ return filteredJson(response.getEntityContent());
+ }
+
+ private static String idProject(String test, String project) {
+ // Expected string of "id": "project_name",
+ return String.format(
+ "\"id\": \"com.googlesource.gerrit.plugins.findowners.FindOwnersIT_" + "%s_%s\",",
+ test, project);
+ }
+
+ private static void verifyRestResult(
+ RestResult result, int voteLevel, int patchset, int changeNumber, boolean addDebugMsg)
+ throws Exception {
+ assertThat(result.minOwnerVoteLevel).isEqualTo(voteLevel);
+ assertThat(result.patchset).isEqualTo(patchset);
+ assertThat(result.change).isEqualTo(changeNumber);
+ assertThat(result.addDebugMsg).isEqualTo(addDebugMsg);
+ if (addDebugMsg) {
+ assertThat(result.dbgmsgs).isNotNull();
+ } else {
+ assertThat(result.dbgmsgs).isNull();
+ }
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/MockedOwnersDb.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/MockedOwnersDb.java
deleted file mode 100644
index fd24a9c..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/MockedOwnersDb.java
+++ /dev/null
@@ -1,71 +0,0 @@
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.findowners;
-
-import static com.google.common.truth.Truth.assertThat;
-import com.googlesource.gerrit.plugins.findowners.Util.String2StringSet;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
-import java.util.Collection;
-import org.junit.Test;
-
-/** Mocked OwnersDb to test Parser class */
-public class MockedOwnersDb extends OwnersDb {
- private String savedData;
- String2StringSet mockedFile2Owners;
-
- public MockedOwnersDb() {
- super(null);
- resetData();
- mockedFile2Owners = new String2StringSet();
- }
-
- void resetData() {
- savedData = "";
- stopLooking = new StringSet();
- }
-
- void appendSavedData(String s) {
- savedData += s;
- }
-
- String getSavedData() {
- return savedData;
- }
-
- StringSet getStopLooking() {
- return stopLooking;
- }
-
- @Override
- void addOwnerPathPair(String s1, String s2) {
- savedData += "s1:" + s1 + "\ns2:" + s2 + "\n";
- }
-
- @Override
- String2StringSet findOwners(Collection<String> files) {
- return mockedFile2Owners;
- }
-
- @Test
- public void defaultTest() {
- // Trivial test of default OwnersDb members.
- assertThat(revision).isEqualTo("");
- assertThat(dir2Globs.size()).isEqualTo(0);
- assertThat(owner2Paths.size()).isEqualTo(0);
- assertThat(path2Owners.size()).isEqualTo(0);
- assertThat(readDirs.size()).isEqualTo(0);
- assertThat(stopLooking.size()).isEqualTo(0);
- }
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/MockedServer.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/MockedServer.java
deleted file mode 100644
index da5df7e..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/MockedServer.java
+++ /dev/null
@@ -1,154 +0,0 @@
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.findowners;
-
-import static com.google.common.truth.Truth.assertThat;
-import com.google.gerrit.reviewdb.client.Change.Status;
-import com.google.gerrit.server.change.RevisionResource;
-import com.google.gson.JsonArray;
-import com.googlesource.gerrit.plugins.findowners.Util.String2Integer;
-import com.googlesource.gerrit.plugins.findowners.Util.String2String;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
-import java.util.Collection;
-import org.eclipse.jgit.lib.Repository;
-import org.junit.Test;
-
-/** Mocked server to test Action, Checker, OwnersDb classes. */
-public class MockedServer extends Server {
-
- int minOwnerVoteLevel;
- boolean addDebugMsg;
- boolean traceServer;
- boolean reportSyntaxError;
- boolean exemptFromOwnerApproval;
- StringSet changedFiles;
- String2Integer votes;
- JsonArray reviewers;
- MockedOwnersDb ownersDb;
- Status status;
- String branchRevision;
- String2String dir2owners; // map from a directory path to OWNERS content
-
- public MockedServer() {
- change = 12;
- patchset = 3;
- project = "projectA";
- branch = "master";
- error = null;
- minOwnerVoteLevel = 1;
- addDebugMsg = true;
- traceServer = true;
- reportSyntaxError = true;
- exemptFromOwnerApproval = false;
- changedFiles = new StringSet();
- String[] sampleFiles = {"./README", "./d1/test.c", "./d2/t.txt"};
- for (String file : sampleFiles) {
- changedFiles.add(file);
- }
- votes = new String2Integer();
- reviewers = new JsonArray();
- ownersDb = new MockedOwnersDb();
- status = Status.NEW;
- branchRevision = "13579abcdef";
- dir2owners = new String2String();
- }
-
- @Override
- int getMinOwnerVoteLevel() {
- return minOwnerVoteLevel;
- }
-
- @Override
- boolean getAddDebugMsg() {
- return addDebugMsg;
- }
-
- @Override
- boolean traceServerMsg() {
- return traceServer;
- }
-
- @Override
- boolean getReportSyntaxError() {
- return reportSyntaxError;
- }
-
- @Override
- boolean isExemptFromOwnerApproval() {
- return exemptFromOwnerApproval;
- }
-
- @Override
- void setChangeId(String url, int change) {}
-
- @Override
- String setPatchId(String patchsetNum) {
- return null;
- }
-
- @Override
- Collection<String> getChangedFiles() {
- return changedFiles;
- }
-
- @Override
- String2Integer getVotes() {
- return votes;
- }
-
- @Override
- JsonArray getReviewers() {
- return reviewers;
- }
-
- @Override
- Status getStatus(RevisionResource resource) {
- return status;
- }
-
- @Override
- String getOWNERS(String dir, Repository repository, String url,
- String project, String branch) {
- String content = dir2owners.get(dir);
- return (null == content ? "" : content);
- }
-
- @Override
- String getBranchRevision(Repository repository, String url,
- String project, String branch) {
- return branchRevision;
- }
-
- @Override
- OwnersDb getCachedOwnersDb() {
- return ownersDb;
- }
-
- @Test
- public void genDebugMsgTest() {
- // Important because real server.traceServerMsg() is usually false.
- String expected =
- "\n## change=12, patchset=3, project=projectA, branch=master"
- + "\n## changedFiles=" + changedFiles
- + "\nnumOwners=0, minVoteLevel=1"
- + ", approvals=" + getVotes();
- assertThat(genDebugMsg(ownersDb)).isEqualTo(expected);
- url = "http://localhost:8081/";
- String expected2 = "\n## url=" + url + expected;
- assertThat(genDebugMsg(ownersDb)).isEqualTo(expected2);
- }
-
- // TODO: use a mocked Repository to test getRepositoryFile
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnerWeightsTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnerWeightsTest.java
index ef28a89..183a222 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnerWeightsTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnerWeightsTest.java
@@ -15,15 +15,20 @@
package com.googlesource.gerrit.plugins.findowners;
import static com.google.common.truth.Truth.assertThat;
+
import com.googlesource.gerrit.plugins.findowners.OwnerWeights.WeightComparator;
-import com.googlesource.gerrit.plugins.findowners.Util.Owner2Weights;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
/** Test OwnerWeights class */
+@RunWith(JUnit4.class)
public class OwnerWeightsTest {
- private OwnerWeights createOwnerWeights(int[] counts) {
+ private static OwnerWeights createOwnerWeights(int[] counts) {
OwnerWeights obj = new OwnerWeights();
for (int i = 0; i < counts.length; i++) {
for (int j = 0; j < counts[i]; j++) {
@@ -65,7 +70,7 @@
assertThat(objX2.encodeLevelCounts()).isEqualTo("[0+2+1]");
assertThat(objX3.encodeLevelCounts()).isEqualTo("[0+2+3]");
assertThat(objX4.encodeLevelCounts()).isEqualTo("[1+1+1]");
- Owner2Weights map = new Owner2Weights();
+ Map<String, OwnerWeights> map = new HashMap<>();
map.put("objX1", objX1);
map.put("objX2", objX2);
map.put("objX3", objX3);
@@ -80,11 +85,11 @@
WeightComparator comp = new WeightComparator(map);
// comp.compare(A,B) < 0, if A has order before B
// comp.compare(A,B) > 0, if A has order after B
- assertThat(comp.compare("objX1", "objX2") > 0).isEqualTo(true);
- assertThat(comp.compare("objX3", "objX2") < 0).isEqualTo(true);
- assertThat(comp.compare("objX3", "objX4") > 0).isEqualTo(true);
+ assertThat(comp.compare("objX1", "objX2")).isGreaterThan(0);
+ assertThat(comp.compare("objX3", "objX2")).isLessThan(0);
+ assertThat(comp.compare("objX3", "objX4")).isGreaterThan(0);
assertThat(comp.compare("objX3", "objX3")).isEqualTo(0);
- assertThat(comp.compare("objX4", "objX0") > 0).isEqualTo(true);
- assertThat(comp.compare("objX0", "objX4") < 0).isEqualTo(true);
+ assertThat(comp.compare("objX4", "objX0")).isGreaterThan(0);
+ assertThat(comp.compare("objX0", "objX4")).isLessThan(0);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersDbTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersDbTest.java
deleted file mode 100644
index df3c7fd..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/OwnersDbTest.java
+++ /dev/null
@@ -1,50 +0,0 @@
-// Copyright (C) 2017 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.googlesource.gerrit.plugins.findowners;
-
-import static com.google.common.truth.Truth.assertThat;
-import org.junit.Before;
-import org.junit.Test;
-
-/** Test OwnersDb class */
-public class OwnersDbTest {
- private MockedServer server;
-
- @Before
- public void setUp() {
- server = new MockedServer();
- }
-
- @Test
- public void ctorTest() {
- // TODO: test getNumOwners, etc.
- }
-
- @Test
- public void addOwnerPathPairTest() {
- // TODO: test addOwnerPathPair
- }
-
- @Test
- public void addFileTest() {
- // TODO: test addFile
- assertThat(1 + 1).isEqualTo(2);
- }
-
- @Test
- public void findOwnersTest() {
- // TODO: test findOwners
- }
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java
index 4488512..6f0ca72 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/ParserTest.java
@@ -15,56 +15,58 @@
package com.googlesource.gerrit.plugins.findowners;
import static com.google.common.truth.Truth.assertThat;
+
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
/** Test Parser class */
+@RunWith(JUnit4.class)
public class ParserTest {
- private MockedOwnersDb db;
-
@Before
public void setUp() {
- db = new MockedOwnersDb();
+ Config.setReportSyntaxError(true);
}
- private String mockedTestDir() {
+ private static String mockedTestDir() {
return "./d1/d2/";
}
- private void testLine(String line) {
- db.resetData();
- String result = Parser.parseLine(db, mockedTestDir(), "OWNERS", line, 3);
- db.appendSavedData((result != null) ? (result + line) : line);
+ private static Parser.Result testLine(String line) {
+ Parser.Result result = new Parser.Result();
+ Parser.parseLine(result, mockedTestDir(), "OWNERS", line, 3);
+ return result;
}
- private String testLineOwnerPath(String line, String s1) {
- return testLineOwnerPath(line, s1, mockedTestDir());
- }
-
- private String testLineOwnerPath(String line, String s1, String s2) {
- // expected db.savedData created by testLine(line)
- // followed by call to addOwnerPathPair(s1, s2)
- return "s1:" + s1 + "\ns2:" + s2 + "\n" + line;
- }
-
- private String testLineWarningMsg(String line) {
+ private static String testLineWarningMsg(String line) {
// expected warning message created by testLine(line)
return Parser.warningMsg("OWNERS", 3, "ignored", line);
}
- private String testLineErrorMsg(String line) {
+ private static String testLineErrorMsg(String line) {
// expected error message created by testLine(line)
return Parser.errorMsg("OWNERS", 3, "ignored unknown line", line);
}
@Test
+ public void emptyParserResult() {
+ Parser.Result result = new Parser.Result();
+ assertThat(result.stopLooking).isFalse();
+ assertThat(result.warnings).isEmpty();
+ assertThat(result.errors).isEmpty();
+ assertThat(result.owner2paths).isEmpty();
+ }
+
+ @Test
public void badLineTest() {
- String[] lines = {"actor", "a@b@c", "**", "per-files *.gyp",
- "a@b.com@c.com #..."};
+ String[] lines = {"actor", "a@b@c", "**", "per-files *.gyp", "a@b.com@c.com #..."};
for (String s : lines) {
- testLine(s);
- String expected = testLineErrorMsg(s) + s;
- assertThat(db.getSavedData()).isEqualTo(expected);
+ Parser.Result result = testLine(s);
+ assertThat(result.warnings).isEmpty();
+ assertThat(result.errors).hasSize(1);
+ String expected = testLineErrorMsg(s);
+ assertThat(result.errors.get(0)).isEqualTo(expected);
}
}
@@ -72,20 +74,24 @@
public void commentLineTest() {
String[] lines = {"", " ", "# comment #data", "#any", " # comment"};
for (String s : lines) {
- testLine(s);
- assertThat(db.getSavedData()).isEqualTo(s);
+ Parser.Result result = testLine(s);
+ assertThat(result.stopLooking).isFalse();
+ assertThat(result.warnings).isEmpty();
+ assertThat(result.errors).isEmpty();
+ assertThat(result.owner2paths).isEmpty();
}
}
@Test
public void emailLineTest() {
- String[] lines = {"a_b-c3@google.com", " x.y.z@gmail.com # comment",
- "*", " * # any user"};
+ String[] lines = {"a_b-c3@google.com", " x.y.z@gmail.com # comment", "*", " * # any user"};
String[] emails = {"a_b-c3@google.com", "x.y.z@gmail.com", "*", "*"};
for (int i = 0; i < lines.length; i++) {
- testLine(lines[i]);
- String expected = testLineOwnerPath(lines[i], emails[i]);
- assertThat(db.getSavedData()).isEqualTo(expected);
+ Parser.Result result = testLine(lines[i]);
+ assertThat(result.owner2paths).hasSize(1);
+ String[] paths = result.owner2paths.get(emails[i]).toArray(new String[1]);
+ assertThat(paths.length).isEqualTo(1);
+ assertThat(paths[0]).isEqualTo(mockedTestDir());
}
}
@@ -94,53 +100,49 @@
// file: directive is not implemented yet.
String[] lines = {"file://owners", " file: //d1/owner", "file:owner #"};
for (String s : lines) {
- testLine(s);
- String expected = testLineWarningMsg(s) + s;
- assertThat(db.getSavedData()).isEqualTo(expected);
+ Parser.Result result = testLine(s);
+ String expected = testLineWarningMsg(s);
+ assertThat(result.warnings).hasSize(1);
+ assertThat(result.warnings.get(0)).isEqualTo(expected);
}
}
@Test
public void noParentLineTest() {
- String[] lines = {"set noparent", " set noparent",
- "set noparent # comment"};
+ String[] lines = {"set noparent", " set noparent", "set noparent # comment"};
for (String line : lines) {
- db.resetData();
- assertThat(db.stopLooking.size()).isEqualTo(0);
+ Parser.Result result = testLine(line);
+ assertThat(result.stopLooking).isTrue();
testLine(line);
- assertThat(db.stopLooking.size()).isEqualTo(1);
- assertThat(db.stopLooking.contains(mockedTestDir())).isEqualTo(true);
- assertThat(db.getSavedData()).isEqualTo(line);
}
}
@Test
public void perFileGoodDirectiveTest() {
- String[] directives = {"abc@google.com#comment",
- " *# comment",
- " xyz@gmail.com # comment"};
+ String[] directives = {"abc@google.com#comment", " *# comment", " xyz@gmail.com # comment"};
String[] emails = {"abc@google.com", "*", "xyz@gmail.com"};
for (int i = 0; i < directives.length; i++) {
String line = "per-file *test*.java=" + directives[i];
- testLine(line);
- String expected =
- testLineOwnerPath(line, emails[i], mockedTestDir() + "*test*.java");
- assertThat(db.getSavedData()).isEqualTo(expected);
+ Parser.Result result = testLine(line);
+ String[] paths = result.owner2paths.get(emails[i]).toArray(new String[1]);
+ assertThat(paths.length).isEqualTo(1);
+ assertThat(paths[0]).isEqualTo(mockedTestDir() + "*test*.java");
}
}
@Test
public void perFileBadDirectiveTest() {
- // TODO: test "set noparent" after perf-file.
- String[] directives =
- {"file://OWNERS", " ** ", "a b@c .co", "a@b@c #com", "a.<b>@zc#"};
- String[] errors =
- {"file://OWNERS", "**", "a", "a@b@c", "a.<b>@zc"};
+ String[] directives = {
+ "file://OWNERS", " ** ", "a b@c .co", "a@b@c #com", "a.<b>@zc#", " set noparent "
+ };
+ String[] errors = {"file://OWNERS", "**", "a b@c .co", "a@b@c", "a.<b>@zc", "set noparent"};
for (int i = 0; i < directives.length; i++) {
String line = "per-file *test*.c=" + directives[i];
- testLine(line);
- String expected = testLineErrorMsg(errors[i]) + line;
- assertThat(db.getSavedData()).isEqualTo(expected);
+ Parser.Result result = testLine(line);
+ String expected = testLineErrorMsg(errors[i]);
+ assertThat(result.warnings).isEmpty();
+ assertThat(result.errors).hasSize(1);
+ assertThat(result.errors.get(0)).isEqualTo(expected);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/findowners/UtilTest.java b/src/test/java/com/googlesource/gerrit/plugins/findowners/UtilTest.java
index 96b29a1..bf7c8a7 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/findowners/UtilTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/findowners/UtilTest.java
@@ -15,106 +15,93 @@
package com.googlesource.gerrit.plugins.findowners;
import static com.google.common.truth.Truth.assertThat;
-import com.googlesource.gerrit.plugins.findowners.Util.Owner2Weights;
-import com.googlesource.gerrit.plugins.findowners.Util.String2Integer;
-import com.googlesource.gerrit.plugins.findowners.Util.String2String;
-import com.googlesource.gerrit.plugins.findowners.Util.String2StringSet;
-import com.googlesource.gerrit.plugins.findowners.Util.StringSet;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
/** Test Util class */
+@RunWith(JUnit4.class)
public class UtilTest {
@Test
public void getOwner2WeightsTest() {
- Owner2Weights m = new Owner2Weights();
- assertThat(m.size()).isEqualTo(0);
- assertThat(m.get("s1")).isEqualTo(null);
+ Map<String, OwnerWeights> m = new HashMap<>();
+ assertThat(m).isEmpty();
+ assertThat(m.get("s1")).isNull();
OwnerWeights v1 = new OwnerWeights();
OwnerWeights v2 = new OwnerWeights();
m.put("s1", v1);
assertThat(m.get("s1")).isEqualTo(v1);
// compare OwnerWeights by reference
assertThat(m.get("s1")).isNotEqualTo(v2);
- assertThat(m.get("s2")).isEqualTo(null);
- assertThat(m.size()).isEqualTo(1);
+ assertThat(m.get("s2")).isNull();
+ assertThat(m).hasSize(1);
}
@Test
public void getString2IntegerTest() {
- String2Integer m = new String2Integer();
- assertThat(m.size()).isEqualTo(0);
- assertThat(m.get("s1")).isEqualTo(null);
+ Map<String, Integer> m = new HashMap<>();
+ assertThat(m).isEmpty();
+ assertThat(m.get("s1")).isNull();
Integer v1 = 3;
Integer v2 = 3;
m.put("s1", v1);
assertThat(m.get("s1")).isEqualTo(v1);
// compare Integer by value
assertThat(m.get("s1")).isEqualTo(v2);
- assertThat(m.get("s2")).isEqualTo(null);
- assertThat(m.size()).isEqualTo(1);
+ assertThat(m.get("s2")).isNull();
+ assertThat(m).hasSize(1);
}
@Test
public void getString2StringTest() {
- String2String m = new String2String();
- assertThat(m.size()).isEqualTo(0);
- assertThat(m.get("s1")).isEqualTo(null);
+ Map<String, String> m = new HashMap<>();
+ assertThat(m).isEmpty();
+ assertThat(m.get("s1")).isNull();
String v1 = "x";
String v2 = "x";
m.put("s1", v1);
assertThat(m.get("s1")).isEqualTo(v1);
// compare String by value
assertThat(m.get("s1")).isEqualTo(v2);
- assertThat(m.get("s2")).isEqualTo(null);
- assertThat(m.size()).isEqualTo(1);
+ assertThat(m.get("s2")).isNull();
+ assertThat(m).hasSize(1);
}
@Test
public void getString2StringSetTest() {
- String2StringSet m = new String2StringSet();
- assertThat(m.size()).isEqualTo(0);
- assertThat(m.get("s1")).isEqualTo(null);
- StringSet v1 = new StringSet();
- StringSet v2 = new StringSet();
- assertThat(v1.size()).isEqualTo(0);
+ Map<String, Set<String>> m = new HashMap<>();
+ assertThat(m).isEmpty();
+ assertThat(m.get("s1")).isNull();
+ Set<String> v1 = new HashSet<>();
+ Set<String> v2 = new HashSet<>();
+ assertThat(v1).isEmpty();
v1.add("x");
v1.add("y");
v2.add("y");
v2.add("x");
- assertThat(v1.size()).isEqualTo(2);
+ assertThat(v1).hasSize(2);
m.put("s1", v1);
assertThat(m.get("s1")).isEqualTo(v1);
- // compare StringSet by value
+ // compare Set<String> by value
assertThat(m.get("s1")).isEqualTo(v2);
- assertThat(m.get("s2")).isEqualTo(null);
- assertThat(m.size()).isEqualTo(1);
+ assertThat(m.get("s2")).isNull();
+ assertThat(m).hasSize(1);
}
@Test
public void addStringSetTest() {
- StringSet s = new StringSet();
- assertThat(s.size()).isEqualTo(0);
+ Set<String> s = new HashSet<>();
+ assertThat(s).isEmpty();
s.add("s1");
- assertThat(s.contains("s1")).isEqualTo(true);
- assertThat(s.contains("s2")).isEqualTo(false);
- assertThat(s.size()).isEqualTo(1);
- }
-
- @Test
- public void normalizeURLTest() {
- String otherURL = "other:///something///else";
- String normalOtherURL = "other://something///else";
- String normalURL = "http://www.google.com:8080/plugins";
- String badURL = "http:///www.google.com:8080/plugins";
- String localURL = "http://localhost:8080/plugins";
- String badLocalURL = "http:///localhost:8080/plugins";
- // Allow other URL protocols, although we might need only http for now.
- assertThat(Util.normalizeURL(otherURL)).isEqualTo(normalOtherURL);
- assertThat(Util.normalizeURL(normalURL)).isEqualTo(normalURL);
- assertThat(Util.normalizeURL(badURL)).isEqualTo(normalURL);
- assertThat(Util.normalizeURL(localURL)).isEqualTo(localURL);
- assertThat(Util.normalizeURL(badLocalURL)).isEqualTo(localURL);
+ assertThat(s.contains("s1")).isTrue();
+ assertThat(s.contains("s2")).isFalse();
+ assertThat(s).hasSize(1);
}
@Test
@@ -125,33 +112,30 @@
assertThat(Util.stripMagicPrefix(")]}'\nxyz")).isEqualTo("xyz");
}
- // TODO: use a mocked HTTP server to test:
- // getHTTP getHTTPJsonObject getHTTPJsonArray getHTTPBase64Content
-
@Test
public void getDirNameTest() {
- String[] files = {"", "./d1/", "d1/d2/f1.c", "./d2/f2.c", "./d1" };
+ String[] files = {"", "./d1/", "d1/d2/f1.c", "./d2/f2.c", "./d1"};
String[] dirs = {null, ".", "d1/d2", "./d2", "."};
for (int i = 0; i < files.length; i++) {
- assertThat(Util.getDirName(files[i])).isEqualTo(dirs[i]);
+ assertThat(Util.getDirName(files[i])).isEqualTo(dirs[i]);
}
}
@Test
public void normalizeFilePathTest() {
- String[] files = {"", "./d1/", "d1/d2/f1.c", "d2/f2/", "d1" };
+ String[] files = {"", "./d1/", "d1/d2/f1.c", "d2/f2/", "d1"};
String[] results = {"./", "./d1/", "./d1/d2/f1.c", "./d2/f2/", "./d1"};
for (int i = 0; i < files.length; i++) {
- assertThat(Util.normalizedFilePath(files[i])).isEqualTo(results[i]);
+ assertThat(Util.normalizedFilePath(files[i])).isEqualTo(results[i]);
}
}
@Test
public void normalizedDirPathTest() {
- String[] files = {"", "./d1/", "d1/d2/f1.c", "./d2/f2.c", "./d1" };
+ String[] files = {"", "./d1/", "d1/d2/f1.c", "./d2/f2.c", "./d1"};
String[] dirs = {null, ".", "./d1/d2", "./d2", "."};
for (int i = 0; i < files.length; i++) {
- assertThat(Util.normalizedDirPath(files[i])).isEqualTo(dirs[i]);
+ assertThat(Util.normalizedDirPath(files[i])).isEqualTo(dirs[i]);
}
}
@@ -160,20 +144,15 @@
String[] yesStrs = {"True", "1", "true", "TRUE", "yes"};
String[] noStrs = {"", "False", "0", "false", "FALSE", "no", "other"};
for (String s : yesStrs) {
- assertThat(Util.parseBoolean(s)).isEqualTo(true);
+ assertThat(Util.parseBoolean(s)).isTrue();
}
for (String s : noStrs) {
- assertThat(Util.parseBoolean(s)).isEqualTo(false);
+ assertThat(Util.parseBoolean(s)).isFalse();
}
}
@Test
- public void sortTest() {
- // TODO: test Util.sort
- }
-
- @Test
- public void newJsonArrayFromStringSetTest() {
- // TODO: test Uril.newJsonArrayFromStringSet
+ public void makeSortedMapTest() {
+ // TODO: test Util.makeSortedMap
}
}
